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

docs: 📝 add guide for Sprout checks #1024

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

martonvago
Copy link
Contributor

Description

This PR adds a guide for checks in Sprout.

I wasn't sure what level of detail to go for, this is ~~medium.

Closes #859

This PR needs an in-depth review.

Checklist

  • Added or updated tests
  • Updated documentation
  • Ran just run-all

@martonvago martonvago self-assigned this Jan 29, 2025
licenses=[sp.LicenseProperties(name="odc-pddl")],
resources=[sp.ResourceProperties()],
)
pprint(sp.check_package_properties(package_properties))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally, I wouldn't even print the output, but then it just prints it unformatted

@martonvago martonvago marked this pull request as ready for review January 29, 2025 17:25
@martonvago martonvago requested a review from a team as a code owner January 29, 2025 17:25
Copy link
Member

@lwjohnst86 lwjohnst86 left a comment

Choose a reason for hiding this comment

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

Super nice!!! Some small suggestions and changes.

docs/guide/checks.qmd Outdated Show resolved Hide resolved
docs/guide/checks.qmd Outdated Show resolved Hide resolved
docs/guide/checks.qmd Outdated Show resolved Hide resolved
docs/guide/checks.qmd Outdated Show resolved Hide resolved
Comment on lines +156 to +165
| raise ExceptionGroup(
| ExceptionGroup: The following checks failed on the properties:
| PackageProperties(name='woolly-dormice', id='123-abc-123', title='Hibernation Physiology of the Woolly Dormouse: A Scoping Review.', description=None, homepage=None, version='1.0.0', created='2014-05-14T05:00:01+00:00', contributors=None, keywords=None, image=None, licenses=[LicenseProperties(name='odc-pddl', path=None, title=None)], resources=[ResourceProperties(name='woolly-dormice-2015', path='https://en.wikipedia.org/wiki/Woolly_dormouse', type=None, title='Body fat percentage in the hibernating woolly dormouse', description=None, sources=None, licenses=None, format=None, mediatype=None, encoding=None, bytes=None, hash=None, dialect=None, schema=None)], sources=None) (3 sub-exceptions)
+-+---------------- 1 ----------------
| seedcase_sprout.core.checks.check_error.CheckError: Error at `$.description` caused by `required`: 'description' is a required property
+---------------- 2 ----------------
| seedcase_sprout.core.checks.check_error.CheckError: Error at `$.resources[0].description` caused by `required`: 'description' is a required property
+---------------- 3 ----------------
| seedcase_sprout.core.checks.check_error.CheckError: Error at `$.resources[0].path` caused by `pattern`: 'path' should contain the resource ID
+------------------------------------
Copy link
Member

Choose a reason for hiding this comment

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

I would love to try to clean this up. I'm not a big fan of Python's error log 😒 Too much text and not that helpful/easy for beginners. We can get into that in the new check-datapackage package though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How the errors are printed is pretty easy to configure. I'm not too sure about the stack trace...

Comment on lines +171 to +185
- "seedcase_sprout.core.checks.check_error.CheckError": the class
representing the error. Check functions will always throw
`CheckError`s.
- "Error at `$.resources[0].description`": the location of the error
in the properties object. `$` corresponds to the topmost layer of
the object (the root); `.resources` points to the `resources` field
of this layer; `[0]` indicates that the error is in the 0th (i.e.
the first, counting from 0) resource properties object; and
`.description` means that the `description` field of this resource
properties is at fault.
- "caused by `required`": the kind of requirement or expectation that
was violated. Here, the expectation is that the field should be
present (i.e. it is a required field).
- "'description' is a required property": a longer, human-readable
explanation of the error.
Copy link
Member

Choose a reason for hiding this comment

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

Very nice description! 👏

- "'description' is a required property": a longer, human-readable
explanation of the error.

## Ignoring errors
Copy link
Member

Choose a reason for hiding this comment

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

This section could actually be it's own guide page, when we move it over into it's own package.

docs/guide/checks.qmd Outdated Show resolved Hide resolved
docs/guide/checks.qmd Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

Guide on using check functions
2 participants