Fix composites not being recorded with desired resolution in deptree #836
Conversation
Codecov Report
@@ Coverage Diff @@
## master #836 +/- ##
==========================================
+ Coverage 83.97% 83.98% +<.01%
==========================================
Files 167 167
Lines 24567 24586 +19
==========================================
+ Hits 20631 20649 +18
- Misses 3936 3937 +1
Continue to review full report at Codecov.
|
| @@ -463,7 +455,8 @@ def _find_dependencies(self, dataset_key, **dfilter): | |||
|
|
|||
| # 0 check if the *exact* dataset is already loaded | |||
| try: | |||
| node = self.getitem(dataset_key) | |||
| dsid = create_filtered_dsid(dataset_key, **dfilter) | |||
| node = self.getitem(dsid) | |||
There was a problem hiding this comment.
I'm not sure about this. Checking if the exact dataset is meant to see if an exact DatasetID exists. I don't think it should be modified by dfilter parameters.
There was a problem hiding this comment.
Well, it's the point of my resolution PRs that the requested dataset with the correct resolution should be returned. Or is that not the point of the resolution keyword in Scene.load ?
There was a problem hiding this comment.
I mean that passing resolution and a name should be equivalent to passing a full fledged DatasetID right ?
There was a problem hiding this comment.
Yes, but if something gets to _find_dependencies and passes a DatasetID that means that's the exact ID that should be used. However, I can see that if that DatasetID (in composite loading, etc) hasn't been updated with the dfilter then it would likely return the wrong resolution.
The problem, from what I can tell, with the way it is implemented now is that if an exact DatasetID is provided to _find_dependencies it's resolution (and other parameters) are overwritten with dfilter even if they were specified exactly by the composite config. Another case would be a user specifying scn.load([DatasetID(name='X', resolution=500), 'Y', 'Z'], resolution=1000), what should happen then?
There was a problem hiding this comment.
ok, I could fix it so that the create_filtered_dsid gives the priority to the existing DatasetID instance if it is filled with other things than None. Would that work ?
mraspaud
force-pushed
the
fix-dep-tree-resolution
branch
from
2f6afa7 to
3e91628
Compare
When a given resolution is requested for loading some composites, the resolution wasn't included in the DatasetIDs recorded in the dependency tree for these composites.
This PR fixes this through making sure the fully filtered dataset ID is created and recorded in the dependency tree.
flake8 satpy