-
Notifications
You must be signed in to change notification settings - Fork 300
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 notes about platform name and sensor to dev guide #3008
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3008 +/- ##
==========================================
+ Coverage 96.08% 96.10% +0.01%
==========================================
Files 377 377
Lines 55155 55162 +7
==========================================
+ Hits 52997 53012 +15
+ Misses 2158 2150 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Pull Request Test Coverage Report for Build 12177642788Details
💛 - Coveralls |
loaded dataset, or return None if the loading was unsuccessful. | ||
loaded dataset, or return None if the loading was unsuccessful. If the reader | ||
is reading satellite data the returned xarray.DataArrays should also have the | ||
attributes ``platform_name`` and ``sensor`` with names according to https://space.oscar.wmo.int/. |
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.
I'm not sure this is true. Sure we try to match OSCAR but they've also changed names in their system in the past. Also we typically have sensor being lowercase and it can be a single string or a set
object. I think we've also had some special cases but don't recall which ones off the top of my head.
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.
For platform name I think that is pretty much ok, but for sensors we've used lowercase and have also replaced /
with -
. As an example AVHRR/3
is avhrr-3
.
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.
While I personally would like it to be like this, I did this PR to get this discussion started.
@pnuu replacing the /
with -
is good which is what space oscar uses. And uppercase/lowercase does not matter as well -
or _
. So at least for the URL it does not matter if you have AVHRR-3
, avhrr-3
, AVHRR_3
or avhrr_3
platform_name
andsensor
attributes according to https://space.oscar.wmo.int/ to documentation