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

BUG: Handle additional data types in rio.reproject #619

Merged
merged 3 commits into from
Dec 19, 2022

Conversation

caldwellst
Copy link
Contributor

@caldwellst caldwellst commented Dec 14, 2022

@snowman2 snowman2 added this to the 0.13.3 milestone Dec 14, 2022
@codecov
Copy link

codecov bot commented Dec 14, 2022

Codecov Report

Base: 96.02% // Head: 95.90% // Decreases project coverage by -0.11% ⚠️

Coverage data is based on head (fffee84) compared to base (0f77e13).
Patch has no changes to coverable lines.

❗ Current head fffee84 differs from pull request most recent head dabc42b. Consider uploading reports for the commit dabc42b to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #619      +/-   ##
==========================================
- Coverage   96.02%   95.90%   -0.12%     
==========================================
  Files          12       12              
  Lines        1735     1735              
==========================================
- Hits         1666     1664       -2     
- Misses         69       71       +2     
Impacted Files Coverage Δ
rioxarray/raster_array.py 98.13% <ø> (ø)
rioxarray/rioxarray.py 96.15% <0.00%> (-0.46%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@snowman2 snowman2 added the bug Something isn't working label Dec 14, 2022
@snowman2 snowman2 requested a review from justingruca December 14, 2022 15:59
@snowman2
Copy link
Member

One option to make this more future proof is to change this:

_NODATA_DTYPE_MAP[dtype_rev[self._obj.dtype.name]]

to:

_NODATA_DTYPE_MAP.get(dtype_rev[self._obj.dtype.name])

Thoughts?

@caldwellst
Copy link
Contributor Author

I think that sounds like a solid idea, particularly as it seems unlikely new data types will be added with corresponding defaults. It may be useful to just set a reminder to check in a set period of time (1 year) if that list has changed in case standard defaults have been added for those that are None.

@snowman2
Copy link
Member

I think that sounds like a solid idea

Mind updating this PR with the change?

@snowman2 snowman2 changed the title Handle additional data types in rio.reproject BUG: Handle additional data types in rio.reproject Dec 19, 2022
@snowman2
Copy link
Member

Thanks @caldwellst 👍

@snowman2 snowman2 merged commit c2f066c into corteva:master Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

_NODATA_DTYPE_MAP causing bug in rio.reproject()
2 participants