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 quickstart tutorial #674

Merged

Conversation

duckontheweb
Copy link
Contributor

@duckontheweb duckontheweb commented Nov 29, 2021

Related Issue(s):

Description:

This PR updates the Quickstart tutorial notebook to be up-to-date with the current version of PySTAC (v1.2.0). It also uses an example catalog that is included in the repository instead of depending on an external catalog. This will help make the tutorial more self-contained, and should make it easier to ensure that the example we are using is in sync with the STAC spec version that PySTAC currently supports.

PR Checklist:

  • Code is formatted (run pre-commit run --all-files)
  • Tests pass (run scripts/test)
  • Documentation has been updated to reflect changes, if applicable
  • 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
Copy link
Contributor Author

@KennSmithDS It would be good to get your feedback on whether this clarifies some of the issues that you ran into when getting started.

@codecov-commenter
Copy link

Codecov Report

Merging #674 (ad8412b) into main (2f42fbc) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #674   +/-   ##
=======================================
  Coverage   94.20%   94.20%           
=======================================
  Files          77       77           
  Lines       11120    11120           
  Branches     1082     1082           
=======================================
  Hits        10476    10476           
  Misses        466      466           
  Partials      178      178           

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 2f42fbc...ad8412b. Read the comment docs.

duckontheweb added a commit to duckontheweb/pystac that referenced this pull request Nov 29, 2021
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.

Content looks great.

Made some comments inline - mostly around how it looks via rendering onto the docs page. Make sure to check how everything looks on the Quickstart section of the rendered docs built locally.

"item = next(local_root_2.get_all_items())\n",
"item.bbox = ['Not a valid bbox']\n",
"item.validate()"
"shutil.rmtree(tmp_dir)"
Copy link
Member

Choose a reason for hiding this comment

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

Should use the ignore_errors=True to avoid error output

"\n",
"[A STAC Catalog](https://github.com/radiantearth/stac-spec/tree/master/catalog-spec) is used to group other STAC objects like Items, Collections, or even other Catalogs.\n",
"\n",
"We will be using a small example catalog adapted from the `geotrellis` [example Landsat Collection](https://github.com/geotrellis/geotrellis-server/tree/977bad7a64c409341479c281c8c72222008861fd/stac-example/catalog/landsat-stac-collection). All STAC Items and Collections can be found in the [docs/example-catalog](./example-catalog) directory of this repo; all Assets are hosted in the Landsat S3 bucket.\n",
Copy link
Member

Choose a reason for hiding this comment

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

Should capitalize project name GeoTrellis. Also perhaps link to project homepage (https://geotrellis.io/)

"\n",
"[STAC Collections](https://github.com/radiantearth/stac-spec/tree/master/collection-spec) are used to group related Items and provide aggregate or summary metadata for those Items.\n",
"\n",
"STAC Catalogs may have many nested layers of Catalogs or Collections within the top-level collection. Our example catalog has one Collection within the main Catalog at ([landsat-8-l1/collection.json](./example-catalog/landsat-8-l1/collection.json). We can list the Collections in a given Catalog using the [`Catalog.get_collections`](https://pystac.readthedocs.io/en/latest/api.html#pystac.Catalog.get_collections) method. This method returns an iterable of PySTAC [`Collection`](https://pystac.readthedocs.io/en/latest/api.html#collection) instances, which we will turn into a `list`."
Copy link
Member

Choose a reason for hiding this comment

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

2 things:

  • There's a stray open paren in the first landsat link that throws off rendering.
  • The nbsphinx can't handle backticks in the brackets for links (seen when rendering the quickstart through make livehtml in the docs folder). You should remove backticks, e.g. [`Catalog.get_collections`] ->[Catalog.get_collections]

@duckontheweb duckontheweb force-pushed the update/665-462-606-quickstart branch from f058c1b to a5f0a9c Compare December 6, 2021 21:23
@duckontheweb duckontheweb merged commit 6e6e987 into stac-utils:main Dec 6, 2021
@duckontheweb duckontheweb deleted the update/665-462-606-quickstart branch December 11, 2021 01:30
@duckontheweb duckontheweb added this to the 1.3.0 milestone Jan 18, 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.

Quickstart and Tutorial documentation out of date Update the quickstart in the docs
4 participants