Apply StacIO instance from root in Catalog.set_root

[duckontheweb]

Related Issue(s):

• Set _stac_io on constructed object if root passed into from_dict #589

Description:

• Adds "public" Catalog.stac_io property that reads/writes to Catalog._stac_io for backwards compatibility.

  Since we are reading and writing to this attribute from outside the owning class in a number of cases it seems appropriate to make it a public property.

• Adds the StacIO instance from root.stac_io to the calling Catalog in Catalog.set_root.

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.

[lossyrob]

Since we are reading and writing to this attribute from outside the owning class in a number of cases it seems appropriate to make it a public property.

The intent with using the underscore was to indicate that the _stac_io property was not part of the public API, i.e. not for user consumption but only for internal library use. This is a different mechanism than the strict public/private-per-class strategy found in typed OO languages. As a Python user, when I see an underscore, I know that I can't rely on that property as a user and should stay away from messing with it. By making this "public" (no underscore), we're committing to this as public API that we should expect users to read from, set into, and generally rely on. Is this the intended change, or do we want that property only to be used within the library code?

[duckontheweb]

The intent with using the underscore was to indicate that the _stac_io property was not part of the public API, i.e. not for user consumption but only for internal library use. This is a different mechanism than the strict public/private-per-class strategy found in typed OO languages. As a Python user, when I see an underscore, I know that I can't rely on that property as a user and should stay away from messing with it. By making this "public" (no underscore), we're committing to this as public API that we should expect users to read from, set into, and generally rely on. Is this the intended change, or do we want that property only to be used within the library code?

That's a good point. If someone were constructing a Catalog instance manually (using __init__), then they might want to set the stac_io attribute directly if they don't want to use the default for some reason. However, it's probably not worth the change until that case actually comes up. I can revert that part of the PR unless there are others who feel it should be a public API.

@gadomski @matthewhanson Are either of you setting Catalog._stac_io directly in stactools or pystac-client?

[gadomski]

@gadomski @matthewhanson Are either of you setting Catalog._stac_io directly in stactools or pystac-client?

pystac-client does, e.g. here: https://github.com/stac-utils/pystac-client/blob/main/pystac_client/client.py#L67. stactools does not.

[duckontheweb]

@gadomski @matthewhanson Are either of you setting Catalog._stac_io directly in stactools or pystac-client?

pystac-client does, e.g. here: https://github.com/stac-utils/pystac-client/blob/main/pystac_client/client.py#L67. stactools does not.

Thanks @gadomski. Based on the fact that the pystac-client use-case is calling a method not present in any of the PySTAC StacIO classes I think it's probably enough of an edge case that it doesn't justify making this a public attribute.

Unless there are other objections or arguments for a public Catalog.stac_io attribute I'll revert that change.

[duckontheweb]

Reverted Catalog.stac_io -> Catalog._stac_io and rebased to resolve merge conflicts. @lossyrob @matthewhanson This is ready for a review.

[codecov-commenter]

Codecov Report

Merging #590 (10b31cd) into main (74e1482) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #590   +/-   ##
=======================================
  Coverage   94.69%   94.69%           
=======================================
  Files          75       75           
  Lines       10777    10790   +13     
  Branches     1053     1054    +1     
=======================================
+ Hits        10205    10218   +13     
  Misses        396      396           
  Partials      176      176           

Impacted Files	Coverage Δ
pystac/catalog.py	91.97% <100.00%> (+0.04%)	⬆️
tests/test_catalog.py	99.18% <100.00%> (+0.01%)	⬆️

---

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 74e1482...10b31cd. Read the comment docs.

[lossyrob]

+1 after the CHANGELOG is updated to remove the public stac_io property mention

[lossyrob]

This entry should be removed

[duckontheweb]

@matthewhanson I'll plan on merging this today unless I hear from you that there are issues.