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 projjson if epsg not available #41

Merged
merged 7 commits into from
Oct 20, 2022

Conversation

clausmichele
Copy link
Contributor

Solves this issue: #40

Let me know if you have any comment or suggestion to improve this PR!

Example image to test the code:
spei30_19970215.zip

@vincentsarago
Copy link
Member

🙏 @clausmichele please feel free to add a test using the file you shared

@clausmichele
Copy link
Contributor Author

@vincentsarago I added a test using the provided file, tests are successfully passing.

rio_stac/stac.py Outdated
except AttributeError:
projjson = None
else:
epsg = src_dst.crs.to_epsg()
Copy link
Member

Choose a reason for hiding this comment

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

Can we do something like 👇

projjson = None
epsg = None
if src_dst.crs is not None:
    epsg = src_dst.crs.to_epsg() if src_dst.crs.is_epsg_code else None
    try:
        projjson = src_dst.crs.to_dict(projjson=True)
    except AttributeError:
        pass

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, thanks for the style improvement :)

@vincentsarago
Copy link
Member

CI errors with TypeError: to_dict() got an unexpected keyword argument 'projjson' because projjson was only added in rasterio>=1.3

This only happens in python3.7 because rasterio 1.3 in not available for this version.

I think we should allow TypeError exception here and in a later PR remove python 3.7 support

rio_stac/stac.py Outdated Show resolved Hide resolved
@vincentsarago vincentsarago merged commit d84d468 into developmentseed:master Oct 20, 2022
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