file modified to create pipeline task cosi_threemlfit #313
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
see 18 files with indirect coverage changes 🚀 New features to boost your workflow:
|
|
Thanks @ldigesu! A couple of things:
Don't hesitate to let me know if you need help with any of this. |
|
The check that is failing is due to missing coverage. In order to pass it you need to write tests that execute the new lines of code you introduced with this commit (the code in cosipy/pipelines). There's some help about the: https://github.com/cositools/cosipy/blob/develop/docs/dev/unit_tests.rst . You can also see multiple other examples from other modules.
The new script is calling cosipy.pipeline.task.task:cosi_threemlfit, which doesn't exist. I think you committed the file .idea/.name instead of task.py. I suppose you are using pycharm. If that's the case, it might work better if you open a pycharm project at the cosipy root directory. You still need to delete the .idea/.name file, it belongs to your IDE, not the repo.
Tks, |
Yes, this is correct. Typically we add a light version of the dataset you used, which while they won't provide scientific-quality results, they are good enough to test the code itself. There are already various files in the cosi.test_data directory. Since you are not adding new functionality to the core library, but a script to run existing functions, I don't expect you to need new files. Please check the existing tests for the spectral fitting code, I suppose you need something very very similar: https://github.com/cositools/cosipy/blob/develop/tests/threeml/test_spectral_fitting.py
Use |
…e so that it does not check per existence of the outputs i.e. only run the task.
# Conflicts: # .gitignore # docs/tutorials/response/DetectorResponse.ipynb # docs/tutorials/source_injector/GRB_source_injector.ipynb # docs/tutorials/source_injector/Point_source_injector.ipynb # setup.py
PR313 followup: resolve conflicts with develop. LD: I do not see any critical change.
Signed-off-by: Israel Martinez <imc@umd.edu>
… beginning to get a more meaningful error message Signed-off-by: Israel Martinez <imc@umd.edu>
Signed-off-by: Israel Martinez <imc@umd.edu>
Signed-off-by: Israel Martinez <imc@umd.edu>
* Make it work for multiple sources * Display the median and error for all free parameters Signed-off-by: Israel Martinez <imc@umd.edu>
Signed-off-by: Israel Martinez <imc@umd.edu>
Signed-off-by: Israel Martinez <imc@umd.edu>
Signed-off-by: Israel Martinez <imc@umd.edu>
Signed-off-by: Israel Martinez <imc@umd.edu>
Signed-off-by: Israel Martinez <imc@umd.edu>
|
@ldigesu I'm sorry we couldn't finish the dicussion on the config file and parameter format yesterday. I still want to provide a concrete example of how all of this would look like. I modified your branch to match my proposed format from the slides I shared yesterday. You can see the changes in #355. We'll keep discussing this next week, but please take a look in the meanwhile and please let me know if there are any concerns. In addition to that, I also noticed a few other things in your PR:
|
PR313 follow-up: proposed config and arguments format. Will keep workin on my branch and commit to the original #313 pull request.
Modified tslice function to work with Time objects in input. Removed unnecessary tslice_ori function and modified task.py accordingly.
… the output files.
|
Hello, @israelmcmc, I merged #355 in my branch, made small modifications to have the time cuts working with Time objects as input, and added the possibility to append a suffix to the names of the output files. The override option does work correctly. I also tested the option of giving tstart, tstop, and output_dir via terminal, and everything works fine. Let me know if you want me to share with you the files that I used to test the time cuts. Let me know if you need anything else. All my changes are committed here. Thanks and cheers, |
|
Thanks, Laura. Looks good to me. I'm merging. |
Hello @israelmcmc. I added the test for the code in tests/pipeline. This is just running the task and checks if the outputs exist in cosipy/test_data. No additional test data were needed in this case. The test works ok for me locally, but when loaded here apparentely it fails, maybe users are not allowed to save outputs? apart from that, there are other tests that are failing, not related no modifications I made. Let me know If you have feedback about this. Thanks and cheers,