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 proj dicts with no key for slicing #129

Merged
merged 6 commits into from Jun 28, 2018
Merged

Ignore proj dicts with no key for slicing #129

merged 6 commits into from Jun 28, 2018

Conversation

mraspaud
Copy link
Member

@mraspaud mraspaud commented Jun 28, 2018

  • Tests added
  • Tests passed
  • Passes git diff origin/master **/*py | flake8 --diff

@coveralls
Copy link

coveralls commented Jun 28, 2018

Coverage Status

Coverage increased (+0.1%) to 85.561% when pulling 2c20dc8 on fix-no-proj into b727fff on master.

@codecov
Copy link

codecov bot commented Jun 28, 2018

Codecov Report

Merging #129 into master will increase coverage by 0.14%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #129      +/-   ##
==========================================
+ Coverage   85.44%   85.59%   +0.14%     
==========================================
  Files          35       35              
  Lines        5957     5984      +27     
==========================================
+ Hits         5090     5122      +32     
+ Misses        867      862       -5
Impacted Files Coverage Δ
pyresample/test/test_kd_tree.py 97.7% <100%> (+0.08%) ⬆️
pyresample/test/test_geometry.py 97.36% <100%> (+0.01%) ⬆️
pyresample/kd_tree.py 92% <100%> (+1.09%) ⬆️
pyresample/geometry.py 78.68% <100%> (ø) ⬆️

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 b727fff...2c20dc8. Read the comment docs.

@mraspaud
Copy link
Member Author

this looks one level down I believe so it should be enough right ? ie in your case, if the variable being resampled has a coord called Time, we now look at the coords of Time itself.

@djhoese
Copy link
Member

djhoese commented Jun 28, 2018

In the case of NUCAPS I believe I was getting errors because the coords had dimensions that the new DataArray did not. In NUCAPS this is something like Number_Of_FORs. So DataArray was complaining because I was making a DataArray with dims ('y', 'x') and coords that had dimensions that didn't exist. Let me see if I can reproduce it locally and I'll give you a self-contained example.

@djhoese
Copy link
Member

djhoese commented Jun 28, 2018

ValueError: coordinate Time has dimensions ('Number_of_CrIS_FORs',), but these are not a subset of the DataArray dimensions ['y', 'x']

@mraspaud
Copy link
Member Author

Isn't Number_of_CrIS_FORs a geographical dimension ?

@djhoese
Copy link
Member

djhoese commented Jun 28, 2018

Technically for NUCAPS it is the only dimension. It is a time and location dimension. Everything is 1D in these NUCAPS EDR files. So Temperature, Time, Longitude, Latitude are all on the same dimension. Pressure is float Pressure(Number_of_CrIS_FORs, Number_of_P_Levels) ;.

@mraspaud
Copy link
Member Author

well in that case it should be renamed to 'x' or 'y' so that pyresample understands it right ?

@djhoese
Copy link
Member

djhoese commented Jun 28, 2018

Not without resampling the data because otherwise your coordinates would have different sizes than your output. We are resampling Temperature (Number_of_CrIS_FORs) to Temperature (y, x).

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.

Works for me with my NUCAPS dataset.

@djhoese djhoese added the bug label Jun 28, 2018
@djhoese djhoese merged commit e404b8d into master Jun 28, 2018
@djhoese djhoese deleted the fix-no-proj branch June 28, 2018 23:56
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.

None yet

3 participants