-
Notifications
You must be signed in to change notification settings - Fork 183
Return xsize and ysize metadata from the ODIMhdf5 reader #535
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
base: master
Are you sure you want to change the base?
Conversation
Added xsize and ysize
dnerini
left a comment
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.
hi @jorahu, many thanks. My main concern is about keeping the output consistent across all readers. If you think adding the xsize and ysize metadata would be valuable for the users, then why not doing it for all readers? As you say, these are easily computed on the fly from the other metadata...
| | xsize | grid size in x-direction | | ||
| +------------------+----------------------------------------------------------+ | ||
| | ysize | grid size in y-direction | | ||
| +------------------+----------------------------------------------------------+ |
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.
Including these would imply providing these two metedata for all available readers
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.
Hey. I now see your point more clearly. First, apologies for this incomplete idea, this is the first time I am using the pull request. This idea might have been better off as a suggestion under Issues probably. Sorry for that. As I am only using the ODIM importer, where this xsize and ysize is a mandatory variable, I thought it's easier to read that parameter from metadata (as it must be there anyway) than to calculate it. But sure, it might not be the case for all the other importer options that pysteps supports. I apparently only looked it from my narrow perspective. As I don't have good knowledge of all the other importers I am certainly not able to fix them all to proceed with this. Probably better to discard this idea at this point as the parameters can be found/calculated from the data itself.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #535 +/- ##
==========================================
- Coverage 83.84% 83.81% -0.04%
==========================================
Files 168 168
Lines 14630 14639 +9
==========================================
+ Hits 12267 12270 +3
- Misses 2363 2369 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ODIMhdf5 reader has been missing xsize and ysize in the importing stage. I know it could be easily calculated from the x and y coordinates and the xpixelsize and ypixelsize, but why add the calculation when the variables are present already in the metadata.