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

Compression changes to eo-learn #731

Merged
merged 10 commits into from
Aug 31, 2023
Merged

Conversation

mlubej
Copy link
Contributor

@mlubej mlubej commented Aug 29, 2023

Changes:

  • compression_level parameter removed
  • compress by default for everything except bbox and timestamps
  • test updates

@mlubej
Copy link
Contributor Author

mlubej commented Aug 31, 2023

@zigaLuksic turns out that backwards compatibility already worked out of the box, since the featureIO objects retained the ability to handled loading of compressed/uncompressed objects

I added back the compression_level parameter as discussed, and added the EODeprecation warning on the save task and the save method. I removed the parameter from the docs, though.

Lastly, I added tests for checking the compression deprecation warning, and augmented a test for checking backwards compatibility loading and proper cleanup if different compressions are present (from before)

@zigaLuksic
Copy link
Collaborator

@zigaLuksic turns out that backwards compatibility already worked out of the box, since the featureIO objects retained the ability to handled loading of compressed/uncompressed objects

Slight correction, the FeatureIO objects retained the GZip capabilities so that we have backwards compatibility ;)
After a while, when this is removed, we'll be able simplify FeatureIO classes

@mlubej
Copy link
Contributor Author

mlubej commented Aug 31, 2023

Also, I wasn't sure if I should keep the compression capabilities in the save_eopatch function (from eolearn/core/eodata_io.py). This one should be internal, so people shouldn't be using it, and the tests might've looked a bit better, but dunno, someone still might use it.

@zigaLuksic
Copy link
Collaborator

Also, I wasn't sure if I should keep the compression capabilities in the save_eopatch function (from eolearn/core/eodata_io.py). This one should be internal, so people shouldn't be using it, and the tests might've looked a bit better, but dunno, someone still might use it.

I see no reason to keep it, since it would control only part of the compression behaviour (for numpy and geodata) which we dont really want to be changed (except for tests)

tests/core/test_eodata_io.py Show resolved Hide resolved
tests/core/test_eodata_io.py Outdated Show resolved Hide resolved
tests/core/test_eodata_io.py Outdated Show resolved Hide resolved
tests/core/test_eodata_io.py Outdated Show resolved Hide resolved
@mlubej mlubej merged commit 5940817 into develop Aug 31, 2023
7 checks passed
@mlubej mlubej deleted the auto-compress-and-remove-parameter branch August 31, 2023 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants