New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add datasets_names attribute to cube models #2782
Conversation
Add datasets_names attribute to each 3d model Adapt serialization so the models.yaml file point to the datasets and remove the models and backgrounds lists from the datasets.yaml file.
Codecov Report
@@ Coverage Diff @@
## master #2782 +/- ##
==========================================
+ Coverage 92.3% 92.31% +0.01%
==========================================
Files 144 144
Lines 16511 16521 +10
==========================================
+ Hits 15240 15251 +11
+ Misses 1271 1270 -1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @QRemy! I've left a few inline comments.
Additionally I think there is a number of places, where we create BackgroundModel
objects internally (like MapDataset.create()
or MapDatasetMaker.run()
), can you make sure to set the correct dataset name to the BackgroundModel
in these cases? And maybe add minimal regression tests to check the agreement of the names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @QRemy, there is one remaining Travis fail in the analysis_mwl.ipynb
notebook, that I'll fix in master.
Add datasets_names attribute to each 3d model.
Adapt serialization so the models.yaml file contains datasets list for each model and remove the models and backgrounds lists from the datasets.yaml file.
If model.datasets_names is not set or set as "all" the model is applied to all datasets.