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

Add plot method and data.py to Esri2020 dataset #405

Merged
merged 8 commits into from
Feb 27, 2022

Conversation

nilsleh
Copy link
Collaborator

@nilsleh nilsleh commented Feb 16, 2022

Since RasterDatasets should have their own plot method per #253, this PR adds a plot method and a data.py file to the Esri2020 dataset.

@github-actions github-actions bot added datasets Geospatial or benchmark datasets testing Continuous integration testing labels Feb 16, 2022
@adamjstewart adamjstewart added this to the 0.3.0 milestone Feb 18, 2022
@calebrob6 calebrob6 closed this Feb 20, 2022
@calebrob6 calebrob6 reopened this Feb 20, 2022
@calebrob6
Copy link
Member

Looks like this is missing data.py

calebrob6
calebrob6 previously approved these changes Feb 23, 2022
calebrob6
calebrob6 previously approved these changes Feb 23, 2022
Copy link
Collaborator

@adamjstewart adamjstewart left a comment

Choose a reason for hiding this comment

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

I don't actually see a data.py file in this PR.

tests/datasets/test_esri2020.py Outdated Show resolved Hide resolved
torchgeo/datasets/esri2020.py Outdated Show resolved Hide resolved
torchgeo/datasets/esri2020.py Outdated Show resolved Hide resolved
@adamjstewart
Copy link
Collaborator

I'm running the test_esri2020.py tests on main (still need to try this PR branch) and they are incredibly slow for some reason.

@adamjstewart
Copy link
Collaborator

Tests are much faster now with this branch, awesome.

When I run the tests, I'm seeing a leftover file that was not tracked by git. I'm assuming you meant to add this to git? I did so in the last commit.

adamjstewart
adamjstewart previously approved these changes Feb 26, 2022
@adamjstewart
Copy link
Collaborator

Tests are missing coverage of the following lines:
Screen Shot 2022-02-26 at 5 09 42 PM

@nilsleh
Copy link
Collaborator Author

nilsleh commented Feb 27, 2022

When I run the tests, I'm seeing a leftover file that was not tracked by git. I'm assuming you meant to add this to git? I did so in the last commit.

I thought, that it was desirable to push as few data files as possible and to keep it minimal with just the zipped data.

@adamjstewart
Copy link
Collaborator

I thought, that it was desirable to push as few data files as possible and to keep it minimal with just the zipped data.

It is, and if you properly copy all zip files to tmp_path before extraction then you shouldn't end up with any new files in the git repo. But since the file is being extracted locally, it ends up creating a new untracked file.

@adamjstewart adamjstewart enabled auto-merge (squash) February 27, 2022 19:47
@adamjstewart adamjstewart merged commit 7bd68e4 into microsoft:main Feb 27, 2022
@adamjstewart adamjstewart mentioned this pull request Jul 11, 2022
yichiac pushed a commit to yichiac/torchgeo that referenced this pull request Apr 29, 2023
* add plot method and data.py

* typo missed period

* forgot data.py

* Remove abc, add versionchanged

* Update esri2020.py

* fixed test and requested changes

* Add uncompressed data file

* add test coverage

Co-authored-by: Caleb Robinson <calebrob6@gmail.com>
Co-authored-by: Adam J. Stewart <ajstewart426@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
datasets Geospatial or benchmark datasets testing Continuous integration testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants