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

Drop x and y indexes before zeroing data in DayNightCompositor #587

Closed
wants to merge 3 commits into from

Conversation

pnuu
Copy link
Member

@pnuu pnuu commented Jan 23, 2019

This PR removes extra indexes from the data arrays before zeroing the data for DayNightCompositor.

@TAlonglong
Copy link
Collaborator

I have tested the bugfix and now it works both when providng an overview or an overview_sun.

Thanks

@djhoese
Copy link
Member

djhoese commented Jan 23, 2019

Is this the fault of the sunz modifier? Would it be possible to have the sunz modifier retain the x/y coords/indexes?

@pnuu
Copy link
Member Author

pnuu commented Jan 24, 2019

The problem is, as far as I can see it, that sunz modifier adds the extra indexes. Maybe it's this line (349) in composites/__init__.py?

coszen = xr.DataArray(cos_zen(vis.attrs["start_time"], lons, lats), dims=['y', 'x'], coords=[vis['y'], vis['x']])

This seems to be copied (by me) also to DayNightCompositor if solar elevation data are not given and needs to be calculated. Otherwise all the compositors/modifiers use the input coordinates: dims=data.dims, coords=data.coords.

@pnuu
Copy link
Member Author

pnuu commented Jan 24, 2019

Using dims=data.dims, coords=data.coords for sunz modifier won't work, as the shapes can be different ((z, y, x) for composite data, (y, x) for SZA data).

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 77.48% when pulling 8617761 on pnuu:bugfix_dnc_overview_sun into d83abd8 on pytroll:master.

@codecov
Copy link

codecov bot commented Jan 24, 2019

Codecov Report

Merging #587 into master will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #587      +/-   ##
==========================================
+ Coverage   77.44%   77.47%   +0.03%     
==========================================
  Files         136      136              
  Lines       19170    19196      +26     
==========================================
+ Hits        14847    14873      +26     
  Misses       4323     4323
Impacted Files Coverage Δ
satpy/composites/__init__.py 64.51% <100%> (+0.35%) ⬆️
satpy/tests/compositor_tests/__init__.py 99.17% <100%> (+0.04%) ⬆️

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 d83abd8...8617761. Read the comment docs.

@djhoese
Copy link
Member

djhoese commented Jan 24, 2019

Using dims=data.dims, coords=data.coords for sunz modifier won't work, as the shapes can be different ((z, y, x) for composite data, (y, x) for SZA data).

But the sunz corrector should never be given RGB data, right? Or am I misunderstanding you.

Also, all SatPy DataArrays should have a y and x dimension, optionally with .coords. Is the problem that it is adding coords not necessarily that it has the dimensions?

@pnuu
Copy link
Member Author

pnuu commented Jan 24, 2019

The data can have the bands dimension (e.g. 'R'), thus making it 3D and incompatible in dims with the strictly 2D SZA array. The problem - as far as I can understand - are the .indexes getting extra things like what is shown in my comment in #584 . Doing the .drop(['x', 'y']) removes those, and after that everything works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DayNightCompositor does not work with eg overview_sun as the day part
4 participants