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 LoveDA dataset #270

Merged
merged 16 commits into from Dec 9, 2021
Merged

Add LoveDA dataset #270

merged 16 commits into from Dec 9, 2021

Conversation

nilsleh
Copy link
Collaborator

@nilsleh nilsleh commented Dec 7, 2021

This is my proposal for adding the LoveDA dataset that Caleb Robinson recommended me to look at as a contribution. This is my first time trying to contribute, so I would welcome any suggestions and criticism. I have based everything off of the code for the gid15 dataset. Running pytest it passed all the tests and had 100% cover on loveda.py.

The dataset differentiates between "rural" and "urban" images so here are some samples respectively from the training split.
trainRuralSamples
trainUrbanSamples

@ghost
Copy link

ghost commented Dec 7, 2021

CLA assistant check
All CLA requirements met.

@ashnair1
Copy link
Collaborator

ashnair1 commented Dec 7, 2021

Hi @nilsleh, it looks like the linters weren't run in your PR. Kindly refer the documentation here to ensure it conforms to torchgeo's style guide

@isaaccorley
Copy link
Collaborator

Hi @nilsleh and thanks for the contribution! I know that the GID-15 didn't have this, but could you add a plot method and show some examples in the PR description? We are working on adding these to all datasets for visually verifying the image and labels were loaded correctly.

@adamjstewart adamjstewart added the datasets Geospatial or benchmark datasets label Dec 7, 2021
@adamjstewart adamjstewart added this to the 0.2.0 milestone Dec 7, 2021
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.

For the test data, you can actually make the images 1x1 or 2x2 to reduce the total file sizes

calebrob6
calebrob6 previously approved these changes Dec 8, 2021
torchgeo/datasets/loveda.py Outdated Show resolved Hide resolved
tests/datasets/test_loveda.py Outdated Show resolved Hide resolved
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.

Everything looks good to me, just a few comments on style

torchgeo/datasets/loveda.py Outdated Show resolved Hide resolved
torchgeo/datasets/loveda.py Show resolved Hide resolved
@adamjstewart adamjstewart changed the title initial commit, adding LoveDA dataset Add LoveDA dataset Dec 8, 2021
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.

Actually, there's one thing missing. Can you add this to docs/api/datasets.rst?

@nilsleh
Copy link
Collaborator Author

nilsleh commented Dec 9, 2021

Actually, there's one thing missing. Can you add this to docs/api/datasets.rst?

I am not sure why the documentation is not picking up the correct return type for get_item and plot method, those two are causing the sphinx test to fail.

@isaaccorley
Copy link
Collaborator

Sometimes the sphinx test will fail. We can re run by closing and opening the PR. If not then maybe an update to packages broke it.

@isaaccorley isaaccorley closed this Dec 9, 2021
@isaaccorley isaaccorley reopened this Dec 9, 2021
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.

Return values do not have names in Sphinx, this is causing the tests to fail

torchgeo/datasets/loveda.py Outdated Show resolved Hide resolved
torchgeo/datasets/loveda.py Outdated Show resolved Hide resolved
torchgeo/datasets/loveda.py Outdated Show resolved Hide resolved
torchgeo/datasets/loveda.py Outdated Show resolved Hide resolved
torchgeo/datasets/loveda.py Outdated Show resolved Hide resolved
torchgeo/datasets/loveda.py Show resolved Hide resolved
adamjstewart
adamjstewart previously approved these changes Dec 9, 2021
@calebrob6 calebrob6 enabled auto-merge (squash) December 9, 2021 19:57
@calebrob6 calebrob6 merged commit 6785683 into microsoft:main Dec 9, 2021
@adamjstewart adamjstewart added datamodules PyTorch Lightning datamodules utilities Utilities for working with geospatial data and removed utilities Utilities for working with geospatial data labels Jan 2, 2022
@nilsleh nilsleh deleted the loveda-dataset branch February 16, 2022 13:22
yichiac pushed a commit to yichiac/torchgeo that referenced this pull request Apr 29, 2023
* initial commit, adding LoveDA dataset

* recommended changes by ashnair1

* corrections from pydocstyle linter

* List[str]

* plotting method added

* linting test changes

* passing test for plotting

* linting adjustments

* smaller fake data of 2x2 and mypy linter changes

* plot only single image and mypy changes

* plot method without expecting batch dimension

* style changes and adding doc

* added lightning data module

* sphinx changes

* doc string changes

* data module in init and fake data for module test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
datamodules PyTorch Lightning datamodules datasets Geospatial or benchmark datasets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants