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

Update docs and tests for Version, Label, and File Info extensions #442

Merged

Conversation

duckontheweb
Copy link
Contributor

@duckontheweb duckontheweb commented Jun 15, 2021

Related Issue(s):

Description:

Version Extension

  • Updates docstrings to be consistent with other extensions

File Info Extension

  • Updates implementation to be consistent with v2.0.0 of the extension spec
  • Makes FileExtension a concrete class on Assets since the spec only defines properties on Assets
  • Updates docstrings to be consistent with other extensions
  • Adds tests to bring coverage up to 95%

Label Extension

  • Updates docstrings to be consistent with other extensions
  • Adds tests to improve coverage to 95%
  • Brings code style in line with other extensions
    Added constants for all property names, changed the bodies of many of the property methods to rely on type checking for input validation, and used get_required and get_opt more consistently.

Other

  • Bumps minimum allowed coverage to 90% since this PR brings it over that threshold.

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.

@codecov-commenter
Copy link

codecov-commenter commented Jun 15, 2021

Codecov Report

Merging #442 (b2fdf5e) into main (46fbaa7) will increase coverage by 0.90%.
The diff coverage is 97.56%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #442      +/-   ##
==========================================
+ Coverage   89.76%   90.67%   +0.90%     
==========================================
  Files          40       40              
  Lines        5150     5105      -45     
==========================================
+ Hits         4623     4629       +6     
+ Misses        527      476      -51     
Impacted Files Coverage Δ
pystac/extensions/eo.py 96.60% <ø> (ø)
pystac/extensions/label.py 96.45% <92.68%> (+10.55%) ⬆️
pystac/extensions/file.py 95.20% <100.00%> (+11.62%) ⬆️
pystac/extensions/version.py 96.07% <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 46fbaa7...b2fdf5e. Read the comment docs.

@duckontheweb duckontheweb requested a review from lossyrob June 15, 2021 01:08
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.

LGTM.

Noticing that the sphinx autodoc output is getting quite messy with the fully qualified types in the signatures. Looked into how to use short names a bit but came up empty.

@duckontheweb
Copy link
Contributor Author

Yeah, I was thinking that, too. I tried setting the add_module_names option to False, but it looks like that does not have any effect on the autodoc extension (and it will end up shortening some other references, which may not be desirable).

Once Sphinx 4.1 hits, it looks like we might be able to take advantage of the autodoc-process-bases event to do some post-processing of the outputs. Their milestone for that release was a few days ago, so it may be out soon. I'll open an issue to track this in case someone else has a better solution.

@duckontheweb
Copy link
Contributor Author

Noticing that the sphinx autodoc output is getting quite messy with the fully qualified types in the signatures. Looked into how to use short names a bit but came up empty.

Tracked in #446

@duckontheweb duckontheweb merged commit 4a6a564 into stac-utils:main Jun 15, 2021
@duckontheweb duckontheweb deleted the fix/326-327-update-proposal-extensions branch June 15, 2021 16:32
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.

3 participants