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

Rename VisionDataset to NonGeoDataset #627

Merged
merged 10 commits into from
Jul 10, 2022
Merged

Rename VisionDataset to NonGeoDataset #627

merged 10 commits into from
Jul 10, 2022

Conversation

adamjstewart
Copy link
Collaborator

@adamjstewart adamjstewart commented Jun 25, 2022

This PR renames the following classes:

  • VisionDataset -> NonGeoDataset
  • VisionClassificationDataset -> NonGeoClassificationDataset

TorchGeo was never intended to be a computer vision library, it is for all geospatial data types. Originally, we mostly had computer vision (satellite/aerial/drone imagery) datasets, but we now have multiple non-vision datasets:

  • Canadian Building Footprints, Open Buildings: vector labels
  • EDDMapS, GBIF, iNaturalist: point labels
  • Aster Global DEM, EU-DEM: topography
  • ADVANCE: audio

Question: should I keep VisionDataset and VisionClassificationDataset aliases with deprecation warnings to help ease the transition? I'm not sure how widely used we are or how stable our users expect us to be. We're still in alpha release, but users don't necessarily realize the implications of that.

@adamjstewart adamjstewart added this to the 0.3.0 milestone Jun 25, 2022
@github-actions github-actions bot added datamodules PyTorch Lightning datamodules datasets Geospatial or benchmark datasets documentation Improvements or additions to documentation samplers Samplers for indexing datasets testing Continuous integration testing labels Jun 25, 2022
@calebrob6
Copy link
Member

calebrob6 commented Jun 29, 2022

I don't mind the rename, although the set of things that are "NonGeoDatasets" is quite large :). I'd like to merge this last before 0.3 so that existing PRs don't have to rebase.

Question: should I keep VisionDataset and VisionClassificationDataset aliases with deprecation warnings to help ease the transition? I'm not sure how widely used we are or how stable our users expect us to be. We're still in alpha release, but users don't necessarily realize the implications of that.

If it is easy to do, maybe. If it is difficult at all, no.

@adamjstewart
Copy link
Collaborator Author

Super easy to do, like 1 line of code to make it work and maybe 4 lines of code to add a deprecation warning.

@calebrob6
Copy link
Member

Cool, then why not!

@adamjstewart
Copy link
Collaborator Author

Think I finally got this working. The only files you need to review are torchgeo/datasets/geo.py and tests/datasets/test_geo.py, everything else is a simple sed replacement. We can merge it now or merge it later, either way we'll need to change the superclass of any datasets that get added in the meantime.

@adamjstewart adamjstewart merged commit ee657ba into main Jul 10, 2022
@adamjstewart adamjstewart deleted the datasets/nongeo branch July 10, 2022 01:28
@adamjstewart adamjstewart mentioned this pull request Jul 11, 2022
yichiac pushed a commit to yichiac/torchgeo that referenced this pull request Apr 29, 2023
* Rename VisionDataset to NonGeoDataset

* Keep VisionDataset but add DeprecationWarning

* mypy fix

* More fixes

* More fixes

* cast types

* Undo cast

* Fix usage in test

* No idea why...

* Update more datasets
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 documentation Improvements or additions to documentation samplers Samplers for indexing datasets testing Continuous integration testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants