Skip to content

Deprecate AugmentationSequential wrapper #2396

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

Merged
merged 3 commits into from
Feb 12, 2025

Conversation

adamjstewart
Copy link
Collaborator

Thoughts on whether we should deprecate this or just remove it? It's been deprecated since 0.4, but also we've been using it continuously since then. We're moving towards a 1.0 release in the future after which we'll need to start formally deprecating things anyway.

@adamjstewart adamjstewart added this to the 0.7.0 milestone Nov 7, 2024
@adamjstewart adamjstewart requested a review from ashnair1 November 7, 2024 09:41
@github-actions github-actions bot added testing Continuous integration testing transforms Data augmentation transforms dependencies Packaging and dependencies labels Nov 7, 2024
@adamjstewart
Copy link
Collaborator Author

It looks like NASA Marine Debris and VHR-10 data modules still use the deprecated wrapper?

@ashnair1
Copy link
Collaborator

ashnair1 commented Nov 7, 2024

Thoughts on whether we should deprecate this or just remove it? It's been deprecated since 0.4, but also we've been using it continuously since then. We're moving towards a 1.0 release in the future after which we'll need to start formally deprecating things anyway.

I prefer just removing it tbh. But I would like to verify kornia's augmentations work as expected for the detection datasets before that.

It looks like NASA Marine Debris and VHR-10 data modules still use the deprecated wrapper?

Yeah, I was planning to have the object detection datasets switch to Kornia's AugmentationSequential in #1978

@adamjstewart
Copy link
Collaborator Author

Gotcha, will mark this as a draft and wait on #1978

@adamjstewart adamjstewart marked this pull request as draft November 7, 2024 11:10
@adamjstewart adamjstewart mentioned this pull request Dec 28, 2024
@github-actions github-actions bot removed the testing Continuous integration testing label Feb 8, 2025
@github-actions github-actions bot added the testing Continuous integration testing label Feb 8, 2025
@adamjstewart adamjstewart marked this pull request as ready for review February 8, 2025 10:54
Copy link
Member

@calebrob6 calebrob6 left a comment

Choose a reason for hiding this comment

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

Slight preference for just removing it as we will definitely want to remove it later

@adamjstewart
Copy link
Collaborator Author

Let's just deprecate it for now. It's only a couple lines of code, it isn't hurting anyone. I think everyone has been using our wrapper up until now, we don't want all of their code to break.

@adamjstewart adamjstewart merged commit b1ed244 into microsoft:main Feb 12, 2025
22 checks passed
@adamjstewart adamjstewart deleted the transforms/deprecate branch February 12, 2025 13:45
adamjstewart added a commit to adamjstewart/torchgeo that referenced this pull request Feb 12, 2025
* Deprecate AugmentationSequential wrapper

* Test deprecation

* Remove tests for deprecated wrapper
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Packaging and dependencies testing Continuous integration testing transforms Data augmentation transforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants