-
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 more reader metadata like long_name and description #887
Conversation
Codecov Report
@@ Coverage Diff @@
## master #887 +/- ##
==========================================
- Coverage 84.62% 84.61% -0.01%
==========================================
Files 169 169
Lines 24996 24996
==========================================
- Hits 21153 21151 -2
- Misses 3843 3845 +2
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #887 +/- ##
==========================================
- Coverage 84.62% 84.61% -0.01%
==========================================
Files 169 169
Lines 24996 24996
==========================================
- Hits 21153 21151 -2
- Misses 3843 3845 +2
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #887 +/- ##
==========================================
+ Coverage 84.62% 84.64% +0.01%
==========================================
Files 169 171 +2
Lines 24996 25035 +39
==========================================
+ Hits 21153 21190 +37
- Misses 3843 3845 +2
Continue to review full report at Codecov.
|
You could use the short name in a dropdown and the long name in titles. But maybe that is too much detail and only long name is sufficient. |
For SIFT, the project manager wanted my version of the |
Could we make Also, maybe the documentation on how to implement a reader needs to be updated to include the new required fields. |
Yes, I think this is reasonable. I think GUIs could probably resort to
Yes, I was planning on it but was waiting for feedback first. |
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.
looks good to me
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.
LGTM, just a minor typo to fix.
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.
One minor thing, otherwise LGTM.
I fixed the change @mraspaud wanted. Not waiting for re-approval... |
I'm creating this as a proof of concept and to continue the discussion from #876 and on slack. The idea is to provide additional human-readable metadata for readers so that they could be documented with more detail, not only in Satpy's documentation but by other applications too. For example, a GUI application using Satpy may want to give the user the option of what reader to use or what readers are available. Having a human-readable name would look "prettier" than the underscored reader name.
This PR, so far, adds short_name, long_name, and description to
abi_l1b
. Thedescription
is formatted as restructuredtext so it could theoretically be pasted to a restructuredtext document for the sphinx docs.Other options:
short_name
. I'm not 100% sure on this since it essentially provides a human readable version of thename
.resources
dictionary mapping "hyperlink text" to a URL. We had talked about doing this for composites so it would be consistent with that and easier to do than typing a ton of URLs in to sentences in the description; especially in rst. This could also mean that the description could stay a plain string which would be nice for usage.Thoughts?
flake8 satpy