-
Notifications
You must be signed in to change notification settings - Fork 122
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
change STACTypeError to create short informative message #1126
change STACTypeError to create short informative message #1126
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1126 +/- ##
==========================================
+ Coverage 90.38% 90.42% +0.03%
==========================================
Files 48 48
Lines 6367 6330 -37
Branches 947 941 -6
==========================================
- Hits 5755 5724 -31
+ Misses 431 428 -3
+ Partials 181 178 -3
☔ View full report in Codecov by Sentry. |
687285b
to
f83c4a3
Compare
f83c4a3
to
e768a3d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adjustments made.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this is a little annoying, but the Catalog.from_file
error message isn't quite what I'd expect. Code (run from root):
from pathlib import Path
from pystac import Catalog
path = (
Path(__file__).parent
/ "tests"
/ "data-files"
/ "catalogs"
/ "invalid-catalog"
/ "catalog.json"
)
Catalog.from_file(str(path))
results in
pystac.errors.STACTypeError: JSON (id = broken_cat) does not represent a STACObject instance.
It's way better than it was, but the best text would end with "...a Catalog instance". Is there any way to get the "final" class type into the message in those STACObject
alternate-constructor methods?
I thought that there was only one case where that should happen, and not that one. Let me take a look. |
b9b6346
to
e988e24
Compare
Related Issue(s):
Description:
This creates a standardized message (which doesn't blow out the console) for
STACTypeError
s.PR Checklist:
pre-commit
hooks pass locallyscripts/test
)