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

Fix area definition YAML not warning on typos #577

Merged
merged 1 commit into from
Jan 29, 2024

Conversation

djhoese
Copy link
Member

@djhoese djhoese commented Jan 3, 2024

If a user has a typo or some other unused parameter in an area YAML definition then pyresample silently ignores it and the user is left confused as to why things aren't working as expected. This PR fixes this by producing a UserWarning in these cases.

I could make this an error but I feel that limits the future flexibility and maintainability of the YAML reading. I could also make a configuration (pyresample.config) parameter that controls whether this is a warnings or error...or maybe just another keyword argument, but I'm not sure we need any more kwargs for these functions.

TODO: I'd like to rework some of the metadata handling in the future area definitions. I might not do that in this PR as it is decently separate. I noticed while working on this that any unused kwarg is put into the future AreaDefinition's .attrs dictionary property. This is error prone as it means that any unrecognized property will go in as "metadata". I'd like to add an explicit attrs: section to the YAML for defining these metadata values.

@djhoese djhoese added the bug label Jan 3, 2024
@djhoese djhoese self-assigned this Jan 3, 2024
Copy link

codecov bot commented Jan 3, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (eaf8367) 94.05% compared to head (26d8dc9) 94.06%.

Files Patch % Lines
pyresample/area_config.py 92.85% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #577   +/-   ##
=======================================
  Coverage   94.05%   94.06%           
=======================================
  Files          90       90           
  Lines       13575    13590   +15     
=======================================
+ Hits        12768    12783   +15     
  Misses        807      807           
Flag Coverage Δ
unittests 94.06% <95.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@coveralls
Copy link

Coverage Status

coverage: 93.656% (+0.007%) from 93.649%
when pulling 26d8dc9 on djhoese:warn-unused-area-params
into eaf8367 on pytroll:main.

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mraspaud mraspaud merged commit 06f6b21 into pytroll:main Jan 29, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

errors in area definition should not be silently ignored
3 participants