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

Support collection list with top level links property #81

Merged
merged 3 commits into from
Jun 28, 2021

Conversation

moradology
Copy link
Collaborator

The OGC standards seem to suggest that a top-level links property should be supported by the /collections endpoint. This PR adds a class to support this slightly more sophisticated list of collections. This change would be useful e.g. here: https://github.com/stac-utils/stac-fastapi/blob/master/stac_fastapi/api/stac_fastapi/api/app.py#L154

@codecov-commenter
Copy link

codecov-commenter commented Jun 4, 2021

Codecov Report

Merging #81 (9386a05) into master (d222206) will decrease coverage by 0.12%.
The diff coverage is 91.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #81      +/-   ##
==========================================
- Coverage   96.38%   96.25%   -0.13%     
==========================================
  Files          19       20       +1     
  Lines         442      454      +12     
==========================================
+ Hits          426      437      +11     
- Misses         16       17       +1     
Flag Coverage Δ
unittests 96.25% <91.66%> (-0.13%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
stac_pydantic/api/collections.py 90.90% <90.90%> (ø)
stac_pydantic/api/__init__.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d222206...9386a05. Read the comment docs.

@lossyrob
Copy link
Member

lossyrob commented Jun 4, 2021

@moradology LGTM, can you run the pre-commit formatter on it so it passes CI?

Copy link
Collaborator

@geospatial-jeff geospatial-jeff left a comment

Choose a reason for hiding this comment

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

lgtm!

@moradology moradology force-pushed the feature/npz/ogc-collections branch from c4cf005 to ab53906 Compare June 8, 2021 13:33
@moradology
Copy link
Collaborator Author

moradology commented Jun 8, 2021

I believe a maintainer will have to pull the trigger on this one as I lack write access to this repo.

Perhaps there's a better place for this question but, as the changes here will be useful for a slight modification to stac-fastapi which makes Collection list endpoint compliant with the STAC spec, how far off from the next release are we?

@moradology
Copy link
Collaborator Author

I think CI should succeed here. black has been run and test coverage is just north of 95%. Perhaps CI needs to be rerun but I think I lack the perms

@lossyrob
Copy link
Member

@moradology CI was re-rerun...perhaps this is an issue with the version of black? pydantic/stac-pydantic/stac_pydantic/api/__init__.py is failing formatting. I also re-ran #90 CI and it failed on black as well. Something seems amiss with your local black matching CI's, maybe the version issue or it's not finding some local configuration

@moradology
Copy link
Collaborator Author

moradology commented Jun 28, 2021

Agreed, I think there's something off with my local version of black because it is not catching anything wrong. just a bunch of unchanged files locally:
image

Investigating now...

@moradology
Copy link
Collaborator Author

@lossyrob Your guess about versions was on the money: black==19.10b0 is an absolute must. I guess major version 20 is less picky

@lossyrob lossyrob merged commit fdb8418 into stac-utils:master Jun 28, 2021
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.

None yet

4 participants