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

Handle zero ranges correctly in regrid operation #2869

Merged
merged 7 commits into from Sep 17, 2018
Merged

Conversation

@philippjfr
Copy link
Member

@philippjfr philippjfr commented Jul 11, 2018

While I fixed the zero range handling in the aggregate operation I seemingly did not do it properly in the regrid operation. Additionally this PR allows Images to contain arrays with a (0, 0) shape, which previously required a hack which would instead store a (1, 1) image.

  • Fixes #2817
  • Adds unit test
@philippjfr
Copy link
Member Author

@philippjfr philippjfr commented Jul 11, 2018

@stevieb Would you mind testing this?

@philippjfr philippjfr force-pushed the rasterize_empty branch from c83c36e to 79c9eba Jul 11, 2018
@stevievb
Copy link

@stevievb stevievb commented Jul 12, 2018

@philippjfr that seemed better in that all of the images don't disappear when you zoom in but now there is a new problem. Once the image is entirely out of the viewport it disappears and never comes back.

@philippjfr
Copy link
Member Author

@philippjfr philippjfr commented Jul 12, 2018

Once the image is entirely out of the viewport it disappears and never comes back.

Odd, I haven't been able to reproduce that. I'll have another look.

@philippjfr philippjfr force-pushed the rasterize_empty branch from 79c9eba to 6239007 Jul 12, 2018
@philippjfr
Copy link
Member Author

@philippjfr philippjfr commented Jul 12, 2018

Nevermind ran into the issue in a completely separate context and fixed it. Thanks for your help @stevievb.

@philippjfr philippjfr force-pushed the rasterize_empty branch from 517d8f8 to 22e3235 Sep 14, 2018
@philippjfr
Copy link
Member Author

@philippjfr philippjfr commented Sep 15, 2018

@jlstevens This is ready to merge once tests pass.

@philippjfr philippjfr force-pushed the rasterize_empty branch from 5bc7e4c to 94fdd67 Sep 15, 2018
@philippjfr philippjfr force-pushed the rasterize_empty branch from 94fdd67 to 6ddd4fc Sep 17, 2018
@jlstevens
Copy link
Contributor

@jlstevens jlstevens commented Sep 17, 2018

Looks good to me. Merging.

@jlstevens jlstevens merged commit a96f75f into master Sep 17, 2018
4 checks passed
4 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.007%) to 83.094%
Details
@philippjfr
s3-reference-data-cache Test data is cached.
Details
@philippjfr philippjfr deleted the rasterize_empty branch Nov 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants