Don't warn about a missing _FillValue for CF coordinate variables - #11524
Open
NoiceHax wants to merge 7 commits into
Open
Don't warn about a missing _FillValue for CF coordinate variables#11524NoiceHax wants to merge 7 commits into
NoiceHax wants to merge 7 commits into
Conversation
NonStringCoder.encode warns when float data is written to an integer dtype without a _FillValue. CF coordinate variables (1D variables named after their dimension) are not allowed to hold missing values, so they never need a _FillValue and the warning is misleading there. Skip the warning when the variable's dims are exactly (name,). Rounding and the dtype cast are untouched, so the encoded output is the same as before. Data variables and multidimensional auxiliary coordinates still warn. Co-authored-by: Claude <noreply@anthropic.com>
for more information, see https://pre-commit.ci
The typos hook rejects the handle in the attribution line, so add it to the people's names allowlist alongside the existing entries, and restore the handle the autofix rewrote. Co-authored-by: Claude <noreply@anthropic.com>
for more information, see https://pre-commit.ci
The typos allowlist entry is in place now, so the hook accepts it. Co-authored-by: Claude <noreply@anthropic.com>
for more information, see https://pre-commit.ci
The autofix was rewriting Hax to Hex in the attribution line. Noice was already allowlisted but Hax was not, so add it and restore the handle. Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Closes #10305.
When a float variable is encoded to an integer dtype,
NonStringCoder.encodewarns that there is no_FillValueto use for NaNs. That warning also fired for CF coordinate variables, meaning 1D variables named after their own dimension. CF says missing data is not allowed in coordinate variables, so those variables are not supposed to carry a_FillValueat all. The warning pushed people toward adding one, which a CF checker then flags.The fix skips the warning when a variable's dims are exactly
(name,), which is the check @kmuehlbauer suggested on the issue. Rounding and the dtype cast are left alone, so the encoded output is the same as before. Data variables and multidimensional auxiliary coordinates still warn, and there are tests for both cases.This covers only the first part of the issue. The other part, what to do when someone sets
_FillValue = Noneand the float data really does contain NaN, is left out on purpose. @kmuehlbauer noted that the cast result there depends on the platform, so it seems better to handle it separately.Checklist
whats-new.rstapi.rstAI Disclosure