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

Added GNDVI and SWI Indices #371

Merged
merged 37 commits into from
Feb 1, 2022
Merged

Added GNDVI and SWI Indices #371

merged 37 commits into from
Feb 1, 2022

Conversation

MATRIX4284
Copy link
Contributor

Added GNDVI and SWI Indices

@github-actions github-actions bot added documentation Improvements or additions to documentation transforms Data augmentation transforms labels Jan 26, 2022
@calebrob6
Copy link
Member

calebrob6 commented Jan 27, 2022

Thanks for consolidating! It looks like the style tests are failing (see https://torchgeo.readthedocs.io/en/latest/user/contributing.html for more info about what is going on here) and unit tests are missing for the new indices (see https://github.com/microsoft/torchgeo/blob/main/tests/transforms/test_indices.py for example tests that you can copy).

Let me know if this doesn't make sense and I can help out!

@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Jan 31, 2022
Copy link
Contributor Author

@MATRIX4284 MATRIX4284 left a comment

Choose a reason for hiding this comment

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

thanks

@MATRIX4284
Copy link
Contributor Author

can this be merged now.

@adamjstewart
Copy link
Collaborator

This still needs unit tests, you should be able to copy and modify the tests for the other indices, see tests/transforms/test_indices.py.

@calebrob6
Copy link
Member

@adamjstewart the coverage for the various indices are actually provided by test_transforms.py, I'm going to add specific tests in test_indices.py so that it is less confusing/ conforms with the standard way of doing things

@github-actions github-actions bot added the testing Continuous integration testing label Jan 31, 2022
@calebrob6
Copy link
Member

Okay I think this is good to go now

@MATRIX4284
Copy link
Contributor Author

thank you

adamjstewart
adamjstewart previously approved these changes Feb 1, 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.

Looks good, couple comments that are optional to address

AppendNDVI,
AppendNDWI,
AppendSWI,
indices,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could replace this with AppendNormalizedDifferenceIndex for consistency

Copy link
Member

Choose a reason for hiding this comment

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

It took me a startling long time to figure out what you meant here


Args:
index_red: index of the VRE1 band, e.g. B5 in Sentinel 2 imagery
index_swir: index of the SWIR2 band, e.g. B11 in Sentinel 2 imagery
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't add examples for other indices. Should we?

Copy link
Member

Choose a reason for hiding this comment

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

Nah I think it is fine

calebrob6
calebrob6 previously approved these changes Feb 1, 2022
@calebrob6 calebrob6 enabled auto-merge (squash) February 1, 2022 01:08
@calebrob6 calebrob6 merged commit 80ee076 into microsoft:main Feb 1, 2022
@calebrob6
Copy link
Member

Thanks again for the contribution @MATRIX4284

@adamjstewart adamjstewart added this to the 0.3.0 milestone Feb 1, 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
* Update indices.py

Added MNDWI to support modification of normalised difference water index (NDWI) to enhance open water features in remotely sensed imagery.

* Update indices.py

* Update indices.py

* Update indices.py

Added a new Index Soil Water Index

* Update indices.py

Style error fixed

* Update indices.py

* SWI addition

* Update indices.py

* Update indices.py

* Update indices.py

* Update indices.py

Changed the comment to fit in the code accordingly

* Update indices.py

modified

* Update indices.py

corrected the citation

* Update indices.py

* Ubuntu CUDA 11.3 docker for torchgeo

* Update Dockerfile_ubuntu_1804_cuda_11_3_cudnn_8_torchgeo

* Delete Dockerfile_ubuntu_1804_cuda_11_3_cudnn_8_torchgeo

* Added Dockerfile

Added Dockerfile

* deleted docker due to wrong issue number

* Added GNDVI Index

Added GNDVI Index

* corrected docstring

* updated refernce index to doi

* Added Triband Normalized Indexes

* Update indices.py

Saved

* Updated GNDVI

* Delete transforms.ipynb

deleting

* Added the notebook changes

* Delete transforms.ipynb

* Added the notebook changes

* Added the spaces

* Update indices.py

* formatting corrected

* Running black

* Reset transforms

* Clean up docstrings

* Adding tests and adding to list of transforms exported by the module

* Consistency

Co-authored-by: Caleb Robinson <calebrob6@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Continuous integration testing transforms Data augmentation transforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants