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.

Sign up for GitHub

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

Jump to bottom

WCS + soilgrids #431

Merged
merged 16 commits into from Oct 30, 2020
Merged

WCS + soilgrids #431

merged 16 commits into from Oct 30, 2020

Conversation

jmilloy
Copy link
Collaborator

@jmilloy jmilloy commented Oct 28, 2020

No description provided.

@jmilloy jmilloy added enhancement New feature or request fixes-and-maintenance labels Oct 28, 2020
@jmilloy
Copy link
Collaborator Author

jmilloy commented Oct 28, 2020 •
edited

@mpu-creare Should I write some tests using an actual WCS server (probably soilgrids)? It will be easy to do, I just don't know if we want to put any tests in the suite that require a network.

@mpu-creare
Copy link
Contributor

I don't want unit tests that require network access. If you could add a few "integration" tests in there (that aren't run every commit) that would be great -- just so we can run at manually every now and again. For the unit tests, see if there's anything that's easy to test based on the coverage report? But don't spend too much time trying to mock a WCS server.

@coveralls
Copy link

coveralls commented Oct 28, 2020 •
edited

Coverage Status

Coverage increased (+1.0%) to 91.392% when pulling 138938e on feature/oswlib-wcs into 13dedab on develop.

@jmilloy
Copy link
Collaborator Author

jmilloy commented Oct 28, 2020

Added some WCS tests for coverage.

@mpu-creare mpu-creare added this to In progress in Priorities via automation Oct 29, 2020
mpu-creare
mpu-creare approved these changes Oct 29, 2020
View reviewed changes
Copy link
Contributor

@mpu-creare mpu-creare left a comment

Choose a reason for hiding this comment

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

You'll see a couple of comments, none are dealbreakers, would be good to address. I'm approving so you can merge at your discretion.

I'm so excited to see this implementation. It's much better than the old way. Nice work.

podpac/core/data/ogc.py Show resolved Hide resolved
podpac/core/data/ogc.py Show resolved Hide resolved
podpac/core/data/ogc.py Show resolved Hide resolved
podpac/core/data/ogc.py Outdated Show resolved Hide resolved
podpac/core/data/ogc.py Show resolved Hide resolved
podpac/core/data/ogc.py Show resolved Hide resolved
podpac/core/data/test/test_wcs.py Show resolved Hide resolved
podpac/datalib/soilgrids.py Show resolved Hide resolved
@jmilloy
FIX: Handle WCS coordinate transforms and uniform lat_lon_alt stacked…
ed9bf4d 
… coords (a rare case, but still). The Coordinates.is_stacked method is stolen from another branch. Tests are added for a few additional WCS eval cases.
@jmilloy
FIX: Only geotiff and geotiff_byte WCS formats are supported at this …
f415315 
…point.
@jmilloy jmilloy merged commit 9e61fc9 into develop Oct 30, 2020
Priorities automation moved this from In progress to Done Oct 30, 2020
@jmilloy jmilloy deleted the feature/oswlib-wcs branch October 30, 2020 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mpu-creare mpu-creare mpu-creare approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request fixes-and-maintenance
Projects
Priorities
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jmilloy @mpu-creare @coveralls