-
Notifications
You must be signed in to change notification settings - Fork 27
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
Improve documentation #99
Conversation
Codecov Report
@@ Coverage Diff @@
## main #99 +/- ##
==========================================
+ Coverage 70.26% 73.02% +2.76%
==========================================
Files 33 33
Lines 2795 2829 +34
==========================================
+ Hits 1964 2066 +102
+ Misses 831 763 -68
Continue to review full report at Codecov.
|
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.
Thanks a lot for taking care of the documentation! Some small comments
Co-authored-by: Martin Raspaud <[email protected]>
Co-authored-by: Martin Raspaud <[email protected]>
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
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.
Thanks for taking care of documentation.
|
||
$> export PYTHONPATH=$PREFIX/lib/python2.7/site-packages:$PYTHONPATH | ||
It is recommended to activate `pre-commit`_ checks. |
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.
Nice, you started using pre-commit
.
and follow the Global Climate Observing System (GCOS) standards for | ||
deriving fundamental climate data records. | ||
|
||
The reflectances are normalized by a factor (as a function of day of a year) |
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 am a bit confused by the implementation of this factor, because it is introduced in https://github.com/pytroll/pygac/blob/main/pygac/reader.py#L510 and passed to the solar calibration, but there it raises a deprecation warning according to https://github.com/pytroll/pygac/blob/main/pygac/calibration.py#L334-L340.
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.
Found this one: #58 (comment) Looks like a future reminder
Thanks for your review @carloshorn ! Very good to double check that the docs match the current implementation! |
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.
Excellent documentation, @sfinkens
I have gone through it and I have no comments.
Thanks a lot!
Thanks for taking the time @abhaydd! |
Improve documentation:
You can preview the updated version here: https://sfinkens.github.io/
Along the way I also included dependencies for development in
setup.py
and fixed a minor bug inReader.get_calibrated_channels
(using timestamps before they are computed).Closes #85, closes #43.