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.

Sign up for GitHub

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

Jump to bottom

allow geo tiff datastore #686

Merged
merged 22 commits into from
May 23, 2022
Merged

allow geo tiff datastore #686

merged 22 commits into from
May 23, 2022

Conversation

TejasMorbagal
Copy link
Contributor

@TejasMorbagal TejasMorbagal commented May 17, 2022 •
edited by forman
Loading

Solves #489

Checklist:

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/source/*
  • Changes documented in CHANGES.md
  • AppVeyor CI passes
  • Test coverage remains or increases (target 100%)

forman and others added 13 commits May 9, 2022 11:41
@forman
Added some code as starting point
3be9a56
@forman
Added example file
3c125fc
@TejasMorbagal
Implemented a class for geotiff to be wrapped in a mldataset
d7aa5b8
@TejasMorbagal
constructing url for tiff file manually and testing it
c1eeafc
@forman
Added more asserts
cfe4959
@forman
Added geotiff extensions and a dedicated example JNB
939def3
@TejasMorbagal
Implemented openers for GeoTiff DataFormat and added the tiff format …
8854b53 
…to _FILENAME_EXT_TO_FORMAT and _FILENAME_EXT_TO_DATA_TYPE_ALIAS
@TejasMorbagal
Implemented Unit Test cases for the Geotiff classes and added an exam…
fc6d63b 
…ple geotiff file
@TejasMorbagal
commented out s3 test on cog to avoid ci failure
2c082d9
@TejasMorbagal
register classes for geotiff
7e33b49
@TejasMorbagal
corrected the file ext to tiff
3ebd905
@TejasMorbagal
added doc strings for the classes and final changes for JNB
cca26a7
@TejasMorbagal
amended the test_io assert to include the tiff datasets
6949cd9
forman
forman requested changes May 18, 2022
View reviewed changes
Copy link
Member

@forman forman left a comment

Choose a reason for hiding this comment

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

Please consider by comments and change requests.

test/core/store/fs/impl/test_cog.py Outdated Show resolved Hide resolved
test/core/store/fs/impl/test_cog.py Outdated Show resolved Hide resolved
test/core/store/fs/impl/test_cog.py Outdated Show resolved Hide resolved
test/core/store/fs/impl/test_cog.py Outdated Show resolved Hide resolved
test/core/store/fs/impl/test_cog.py Outdated Show resolved Hide resolved
xcube/core/store/fs/impl/cog.py Outdated Show resolved Hide resolved
xcube/core/store/fs/impl/dataset.py Outdated Show resolved Hide resolved
xcube/core/store/fs/impl/dataset.py Outdated Show resolved Hide resolved
xcube/core/store/fs/registry.py Outdated Show resolved Hide resolved
examples/serve/demo/README.md Outdated Show resolved Hide resolved
@TejasMorbagal TejasMorbagal marked this pull request as ready for review May 18, 2022 13:03
@TejasMorbagal
- a COG format with multiple overviews and open it as multi-level dat…
9e29c93 
…aset or a dataset for a given overview level

- a GeoTIFF with no overviews and open it as multi-level dataset or as normal dataset
@TejasMorbagal
added sample.tif and allow opening of a mldataset as a GeoTIFF datase…
23a9ba8 
…t with overview
@TejasMorbagal
updated cog ipynb
7499c6f
forman
forman requested changes May 19, 2022
View reviewed changes
Copy link
Member

@forman forman left a comment

Choose a reason for hiding this comment

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

  1. See my suggestions
  2. Leave entry in CHANGES.md
  3. Rename cog.py into geotiff.py and test_cog.py into test_geotiff.py
  4. You are not yet using the reported open parameters tile_size and overview_level. Please do so.

environment.yml Outdated Show resolved Hide resolved
xcube/core/store/fs/impl/cog.py Outdated Show resolved Hide resolved
xcube/core/store/fs/impl/cog.py Outdated Show resolved Hide resolved
xcube/core/store/fs/impl/cog.py Outdated Show resolved Hide resolved
xcube/core/store/fs/impl/dataset.py Outdated Show resolved Hide resolved
xcube/core/store/fs/impl/dataset.py Outdated Show resolved Hide resolved
xcube/core/store/fs/impl/dataset.py Show resolved Hide resolved
xcube/core/store/fs/impl/cog.py Outdated Show resolved Hide resolved
xcube/core/store/fs/impl/cog.py Outdated Show resolved Hide resolved
@forman forman self-requested a review May 23, 2022 11:58
forman
forman approved these changes May 23, 2022
View reviewed changes
Copy link
Member

@forman forman left a comment

Choose a reason for hiding this comment

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

Thanks @TejasMorbagal!

@TejasMorbagal TejasMorbagal merged commit a4c2c17 into master May 23, 2022
@TejasMorbagal TejasMorbagal deleted the TejasMorbagal-489-allow-GeoTiff-datastore branch June 28, 2022 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@forman forman forman approved these changes

Assignees

@TejasMorbagal TejasMorbagal

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@TejasMorbagal @forman