Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Cordex extension and trying to build on higher level abstractions #64
base: master
Are you sure you want to change the base?
Cordex extension and trying to build on higher level abstractions #64
Changes from 12 commits
3c85233
f1c5683
c0bdeb2
3a4b6ef
b7d9b75
98c6420
33d78b9
0212008
04d94df
ac0cec2
03e5738
8cceeab
8b583bb
8db0847
df35431
c6e68e8
d76fdd8
adb1065
fd49161
08ccd42
2fb0eb6
312694c
a847c3f
72a8271
4817d66
2b43022
118810c
d1d838e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Can't the
model_config
be used for that?Otherwise, reuse the same
PrivateAttr
approach, and filter by annotation/field-type?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.
I don't think so, because those were fields I wanted to exclude from the schema validation, but not from the model dump. I was thinking of a case where the schema is strictly prohibiting extra attributes, but I realize this might be a very edgy corner case.
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.
Could be improved with
requests
file-handler, allowing either local or remote URI, but not "blocking" for the PR.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.
I was unsure how to deal with references within a schema if it was not local.
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.
It's possible with the jsonschema library that we're using to have the library resolve remote references with
requests
. See the example here (which useshttpx
notrequests
but the idea is the same).I agree that this can be for another PR though
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.
This will be common to a lot of STAC models, so maybe extract it out to reuse it as base.
Consider using
AnyGeometry | None
instead ofGeoJSONPolygon
(as per STAC Item spec).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.
Should probably be a class method.
As a property, the name feels like an attribute to the current model. However, it seems to be employed as a "generate new UUID" helper method for each STAC Item created.
Also, import at the top.
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.
Typically, it is the extended object that should be returned, such that whatever the extensions provides becomes available properties or helper methods. The item reference can be obtained from
obj_ext.item
.