Skip to content
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 note about datatype in custom reader documentation #1130

Merged
merged 2 commits into from Apr 3, 2020

Conversation

mraspaud
Copy link
Member

@mraspaud mraspaud commented Apr 2, 2020

  • Fully documented

@mraspaud mraspaud added enhancement code enhancements, features, improvements documentation labels Apr 2, 2020
@mraspaud mraspaud requested a review from djhoese as a code owner April 2, 2020 13:16
@mraspaud mraspaud self-assigned this Apr 2, 2020
Copy link
Member

@djhoese djhoese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this include a simple of example of how a user can preserve this? Something like a .where example but using a 32-bit float NaN or something like that? Or mention astype?

@@ -506,6 +506,18 @@ a convenience and are not required to read these formats. In many cases using
the :func:`xarray.open_dataset` function in a custom file handler is a much
better idea.

.. note::
Be careful about the data types of the datasets your reader is returning.
It is easy to let the data being coerced into double precision floats. At the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

being -> be

@coveralls
Copy link

coveralls commented Apr 2, 2020

Coverage Status

Coverage increased (+0.003%) to 89.539% when pulling ebbf2f1 on mraspaud:feature-datatype-note into 11bd31d on pytroll:master.

@codecov
Copy link

codecov bot commented Apr 3, 2020

Codecov Report

Merging #1130 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1130      +/-   ##
==========================================
+ Coverage   89.53%   89.53%   +<.01%     
==========================================
  Files         200      200              
  Lines       29356    29384      +28     
==========================================
+ Hits        26284    26310      +26     
- Misses       3072     3074       +2
Impacted Files Coverage Δ
satpy/scene.py 90.01% <0%> (-0.18%) ⬇️
satpy/tests/reader_tests/test_utils.py 100% <0%> (ø) ⬆️
satpy/tests/reader_tests/test_seviri_l1b_hrit.py 100% <0%> (ø) ⬆️
satpy/tests/reader_tests/test_geos_area.py 100% <0%> (ø) ⬆️
satpy/tests/reader_tests/test_ahi_hsd.py 100% <0%> (ø) ⬆️
satpy/tests/reader_tests/test_hrit_base.py 97.89% <0%> (+0.04%) ⬆️
satpy/readers/utils.py 86.92% <0%> (+0.14%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 11bd31d...ebbf2f1. Read the comment docs.

@ghost
Copy link

ghost commented Apr 3, 2020

Congratulations 🎉. DeepCode analyzed your code in 0.011 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard

Copy link
Member

@djhoese djhoese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. This is reminding me though that I (we?) should more completely document how to handle integer category products and the use of _FillValue. That can be done in a separate PR.

@djhoese djhoese merged commit 2ce07c0 into pytroll:master Apr 3, 2020
@djhoese djhoese removed the enhancement code enhancements, features, improvements label Apr 3, 2020
@djhoese
Copy link
Member

djhoese commented Apr 3, 2020

FYI I removed the enhancement label since this isn't a code change.

@mraspaud mraspaud deleted the feature-datatype-note branch August 17, 2021 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants