Skip to content

Update docs, tests, summaries for stable extensions #372

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

Conversation

duckontheweb
Copy link
Contributor

Related Issue(s): #

Description:

  • Updates API docs for view, projection, and scientific extensions
  • Makes doi an optional argument for ScientificExtension and Publication to match spec
  • Implements summaries for view, projection, and scientific extensions
  • Simplifies summary getter/setter docstrings

stac-fields indicates that JSON Schema format should be used for sci:citation summaries, but using a simple list seemed more appropriate, so that's how I implemented it for now. @m-mohr @matthewhanson Maybe you have more insight around the rationale for using the JSON Schema summaries for that field?

PR Checklist:

  • Code is formatted (run scripts/format)
  • Tests pass (run scripts/test)
  • This PR maintains or improves overall codebase code coverage.
  • Changes are added to the CHANGELOG. See the docs for information about adding to the changelog.

@duckontheweb duckontheweb changed the title 326 327 update stable extensions Update docs, tests, summaries for stable extensions May 28, 2021
@codecov-commenter
Copy link

codecov-commenter commented May 28, 2021

Codecov Report

Merging #372 (0d406d1) into main (c62f0fb) will increase coverage by 0.26%.
The diff coverage is 98.54%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #372      +/-   ##
==========================================
+ Coverage   89.43%   89.69%   +0.26%     
==========================================
  Files          39       39              
  Lines        5063     5154      +91     
==========================================
+ Hits         4528     4623      +95     
+ Misses        535      531       -4     
Impacted Files Coverage Δ
pystac/extensions/label.py 85.89% <ø> (ø)
pystac/extensions/scientific.py 96.15% <96.66%> (+0.21%) ⬆️
pystac/extensions/eo.py 97.34% <100.00%> (+0.01%) ⬆️
pystac/extensions/projection.py 96.63% <100.00%> (+0.52%) ⬆️
pystac/extensions/view.py 97.45% <100.00%> (+1.35%) ⬆️
pystac/extensions/base.py 91.48% <0.00%> (+2.12%) ⬆️
pystac/summaries.py 78.20% <0.00%> (+2.56%) ⬆️

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 c62f0fb...0d406d1. Read the comment docs.

@m-mohr
Copy link
Contributor

m-mohr commented May 28, 2021

Yeah, maybe "v" is better as "s" on citation.

We had the case where we had a regexp for the year number in the citation summary, which would be a use case. But I agree that in general, a list of values is probably the better option.

Opened PR stac-utils/stac-fields#11 for it.

m-mohr added a commit to stac-utils/stac-fields that referenced this pull request May 28, 2021
* Use default behavior (list of values) instead of schema

Origin: stac-utils/pystac#372
Copy link
Member

@lossyrob lossyrob left a comment

Choose a reason for hiding this comment

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

Looking good, couple of suggestions but overall +1

@duckontheweb duckontheweb merged commit 17ee2cc into stac-utils:main Jun 2, 2021
@duckontheweb duckontheweb deleted the 326-327-update-stable-extensions branch June 2, 2021 13:50
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.

4 participants