Fix swath builtin coordinates not being used #1541
Conversation
DeepCode failed to analyze this pull requestSomething went wrong despite trying multiple times, sorry about that. |
|
Thanks! My tests with avhrr and viirs data reading back with Will take a look at the code later. |
| @@ -823,19 +823,31 @@ def _load_dataset_with_area(self, dsid, coords, **kwargs): | |||
| if not file_handlers: | |||
| return | |||
|
|
|||
| area = self._load_dataset_area(dsid, file_handlers, coords, **kwargs) | |||
|
|
|||
| try: | |||
There was a problem hiding this comment.
LGTM, but how will moving this below affect reading other netcdf data? We rely on the tests?
There was a problem hiding this comment.
we do rely on tests yes, and these are failing. I have to take a look at the problem
|
@djhoese because there is no |
Codecov Report
@@ Coverage Diff @@
## master #1541 +/- ##
==========================================
+ Coverage 92.02% 92.03% +0.01%
==========================================
Files 249 249
Lines 36309 36384 +75
==========================================
+ Hits 33413 33486 +73
- Misses 2896 2898 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
satpy/readers/yaml_reader.py
Outdated
| lats = coord | ||
| if lons is None or lats is None: | ||
| raise ValueError('Missing longitude or latitude coordinate: ' + str(coords)) | ||
| return lats, lons |
There was a problem hiding this comment.
Nitpick, the return value is the opposite of the method name. Prefer lon, lats I think.
satpy/readers/yaml_reader.py
Outdated
| if area is not None: | ||
| ds.attrs['area'] = area | ||
| ds = add_crs_xy_coords(ds, area) | ||
| return ds | ||
|
|
||
| @staticmethod | ||
| def _assign_builtin_coords(coords, ds): |
There was a problem hiding this comment.
Thoughts on changing the name to something like _assign_existing_coords or _assign_coords_from_data_array? To me builtin is ambiguous.
| self.assertTrue(np.all(scn_['image0'].data == self.scene['image0'].data)) | ||
| self.assertTrue(np.all(scn_['lat'].data == self.scene['lat'].data)) # lat loaded as dataset | ||
| self.assertTrue(np.all(scn_['image0'].coords['lon'] == self.scene['lon'].data)) # lon loded as coord | ||
| finally: |
There was a problem hiding this comment.
Hm, never seen a try without an except. So any exceptions raise before finally is kind of ignored with this approach? Instead of
except:
pass
There was a problem hiding this comment.
No, the exception isn't ignored, but 'finally' ensures its part of the code is executed before raising the exception.
This PR allows satpy to use longitudes and latitudes provided as coordinates to other dataarrays to create the dataarrays'
areaattribute.Consider for example:
This dataarray contains already populated longitude and latitude coordinates (it is being read from a well-constructed netcdf file), but satpy fails to create the necessary
areaattribute from it.This PR fixes it in a manner that uses the builtin coordinates if no other coordinates are provided from eg the dataset definitions of the reader's yaml file.
flake8 satpy