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

Ignore raw metadata when combining metadata #1663

Merged
merged 3 commits into from May 10, 2021

Conversation

sfinkens
Copy link
Member

@sfinkens sfinkens commented May 6, 2021

When combining metadata from multiple datasets, ignore the raw_metadata attribute. Usually these are nested dictionaries containing numpy arrays. You could compare them using np.testing.assert_equal, but

  • raw metadata from another time/sensor will be different anyway
  • this would slow down the code a little bit, because the raw metadata can be quite large
  • we probably shouldn't use the numpy test module outside the unit tests

So I'd say it's not worth the effort.

@codecov
Copy link

codecov bot commented May 6, 2021

Codecov Report

Merging #1663 (202c6fd) into main (6df2f10) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 202c6fd differs from pull request most recent head 5f747c4. Consider uploading reports for the commit 5f747c4 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1663   +/-   ##
=======================================
  Coverage   92.65%   92.65%           
=======================================
  Files         258      258           
  Lines       37948    37952    +4     
=======================================
+ Hits        35161    35165    +4     
  Misses       2787     2787           
Flag Coverage Δ
behaviourtests 4.83% <0.00%> (-0.01%) ⬇️
unittests 92.79% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
satpy/tests/test_dataset.py 100.00% <ø> (ø)
satpy/dataset/metadata.py 98.73% <100.00%> (+0.06%) ⬆️

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 6df2f10...5f747c4. Read the comment docs.

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.

Thanks for working on this. I had a couple suggestions.

satpy/dataset/metadata.py Outdated Show resolved Hide resolved
satpy/dataset/metadata.py Outdated Show resolved Hide resolved
satpy/tests/test_dataset.py Show resolved Hide resolved
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.

LGTM. Thanks!

@djhoese
Copy link
Member

djhoese commented May 6, 2021

Why did travis and appveyor suddenly show up again in this PR? Did you have an old master/main branch that you based this branch off of?

@sfinkens
Copy link
Member Author

sfinkens commented May 7, 2021

Hmm, that is strange. The branch is based on 6df2f10 which was a couple commits before master was renamed to main (did that in my fork, too)

Copy link
Member

@ameraner ameraner 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 a lot for your help fixing this!

@mraspaud mraspaud merged commit 88575cc into pytroll:main May 10, 2021
@sfinkens sfinkens deleted the fix-combine-raw-metadata branch May 11, 2021 13:43
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.

Cannot load datasets of multiple SEVIRI native files
4 participants