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

add custom validator as input #1320

Merged
merged 5 commits into from
Mar 28, 2024
Merged

Conversation

fmigneault
Copy link
Contributor

@fmigneault fmigneault commented Mar 18, 2024

Related Issue(s):

  • n/a

Description:

Currently, performing validation on STAC objects with extensions that are not directly part of pystac, or that the schema is not yet published at the expected URI defined by the extension, is not convenient or obvious.

In order to call <STACObject>.validate(), one has to either duplicate the operations done in pystac.validation.validate_dict to use their own validator or set it globally with RegisteredValidator.set_validator before calling <STACObject>.validate(). This PR adds a validator input to all the relevant functions to directly provide the validator to use for validation, with a local reference. This makes it easier to manage the "active" validator at the moment the validate method gets called.

PR Checklist:

  • pre-commit hooks pass locally
  • 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.

Copy link
Member

@jsignell jsignell left a comment

Choose a reason for hiding this comment

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

This seems like a nice change! It would be great to have more information in the docstrings and some tests.

pystac/stac_object.py Show resolved Hide resolved
pystac/validation/__init__.py Outdated Show resolved Hide resolved
Copy link

codecov bot commented Mar 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.18%. Comparing base (c0adad3) to head (d096ac0).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1320   +/-   ##
=======================================
  Coverage   91.18%   91.18%           
=======================================
  Files          51       51           
  Lines        7023     7024    +1     
  Branches     1005     1005           
=======================================
+ Hits         6404     6405    +1     
  Misses        444      444           
  Partials      175      175           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fmigneault
Copy link
Contributor Author

@jsignell I will look into adding a test.

@fmigneault fmigneault requested a review from jsignell March 21, 2024 17:57
Copy link
Member

@jsignell jsignell left a comment

Choose a reason for hiding this comment

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

Thanks for adding the test and docs!

pystac/validation/__init__.py Show resolved Hide resolved
@@ -183,6 +183,55 @@ def test_validates_geojson_with_tuple_coordinates(self) -> None:
# Should not raise.
item.validate()

@pytest.mark.vcr()
Copy link
Member

Choose a reason for hiding this comment

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

nice test! If you need vcr then there is probably a cassette file that you need to check in as well. In that case CI will fail. If CI passes then I think you don't need to have pytest.mark.vcr here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not too familiar with VCR. I assume I need the same references that are contained in data-files/item/sample-item.json ? I simply reused that item from another test case.

Copy link
Member

Choose a reason for hiding this comment

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

I took a look and it seems like this decorator is good enough. Don't need a new cassette as well.

@fmigneault fmigneault requested a review from jsignell March 22, 2024 20:41
@fmigneault
Copy link
Contributor Author

@jsignell
Can this be validated again?
I would like to integrate it to complete stac-extensions/ml-aoi#8
Also, would it be possible to get a release tag soon after so I can pin the minimum version?

fmigneault added a commit to stac-extensions/ml-aoi that referenced this pull request Mar 28, 2024
Copy link
Member

@jsignell jsignell left a comment

Choose a reason for hiding this comment

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

This looks good to me! Thanks for your patience.

@jsignell jsignell added this pull request to the merge queue Mar 28, 2024
Merged via the queue into stac-utils:main with commit e4330bc Mar 28, 2024
24 checks passed
fmigneault added a commit to stac-extensions/ml-aoi that referenced this pull request Apr 2, 2024
@fmigneault fmigneault deleted the custom-validator branch April 2, 2024 22:05
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.

2 participants