-
Notifications
You must be signed in to change notification settings - Fork 123
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
Issues 333 334 - fix 1.0.0-rc.1 extensions and 1.0.0-beta.1 schema links #336
Conversation
Hey @jonhealy1, seems like there's a lot of commits here but the final line changes are +12/-8 ... did some things get lost in the shuffle or are some of these changes reverted? |
Hi @lossyrob sorry yes. I was implementing tests to run validation against some of the stac objects we test against for stac-validator. I pulled those changes out and pushed them to a local branch because they are not finished and I wasn't sure if it was something that would be valuable. |
There are only a few minor changes in this pr. |
Ok, gotcha. I'll do a squash commit on this PR so that we can get rid of those extraneous commits in the main history. GitHub Actions seems to be having some issues so we'll wait until we can rerun the CI, but this is looking good to me |
Thank you. Next time I will be more careful. |
@lossyrob I could redo the pr too if it's easier. |
Nah a squash commit will do the trick. If you wanted to you could rebase these commits into one or two and force push to your branch, which would clean this PR up, but I wouldn't worry about it too much! |
@jonhealy1 I think it would make sense to add a test case for an .rc1 STAC item that would fail without this change, so we can guard against changes that might break this in the future. |
@lossyrob Just for reference - name shortcuts became invalid in 1.0.0-rc.2 not 1.0.0-rc.1 https://github.com/radiantearth/stac-spec/blob/master/CHANGELOG.md#removed-1 v1.0.0-rc.2 - 2021-03-30 |
@lossyrob test_identify.py fails because of the 1.0.0-rc.1 example I added to example-info.csv for testing? E AssertionError: False is not true : Failed /Users/comp/Sparkgeo/pystac/tests/data-files/examples/1.0.0-RC1/extended-item.json: tests/serialization/test_identify.py:45: AssertionError |
nevermind - someone put the 1.0.0-beta.2 examples in the 1.0.0-RC1 folder |
E fixture 'test_class' not found ======================================= short test summary info ======================================== |
"1.0.0-RC3/catalog.json","CATALOG","1.0.0-rc.3","" | ||
"1.0.0-RC3/collection-only/collection.json","COLLECTION","1.0.0-rc.3","https://stac-extensions.github.io/eo/v1.0.0/schema.json|https://stac-extensions.github.io/projection/v1.0.0/schema.json|https://stac-extensions.github.io/view/v1.0.0/schema.json" | ||
"1.0.0-RC3/collection.json","COLLECTION","1.0.0-rc.3","https://stac-extensions.github.io/eo/v1.0.0/schema.json|https://stac-extensions.github.io/view/v1.0.0/schema.json" | ||
"1.0.0-RC3/collectionless-item.json","ITEM","1.0.0-rc.3","https://stac-extensions.github.io/eo/v1.0.0/schema.json|https://stac-extensions.github.io/view/v1.0.0/schema.json" | ||
"1.0.0-RC3/core-item.json","ITEM","1.0.0-rc.3","" | ||
"1.0.0-RC3/extended-item.json","ITEM","1.0.0-rc.3","https://stac-extensions.github.io/eo/v1.0.0/schema.json|https://stac-extensions.github.io/projection/v1.0.0/schema.json|https://stac-extensions.github.io/scientific/v1.0.0/schema.json|https://stac-extensions.github.io/view/v1.0.0/schema.json|https://stac-extensions.github.io/remote-data/v1.0.0/schema.json" | ||
"1.0.0-RC3/extensions-collection/collection.json","COLLECTION","1.0.0-rc.3","" | ||
"1.0.0-RC3/extensions-collection/proj-example/proj-example.json","ITEM","1.0.0-rc.3","https://stac-extensions.github.io/eo/v1.0.0/schema.json|https://stac-extensions.github.io/projection/v1.0.0/schema.json" |
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.
Looks like these files don't exist yet, but once that's fixed this looks good to me, too.
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.
Actually, I think these just need to be changed to 1.0.0-RC2
. @jonhealy1 I'm happy to merge in main
and update this if you want. I think this PR is ready to go other than this.
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 think I only added in one rc1 and one rc2 entry. I think the rc3 stuff comes from a previous iteration of what I was building off of? If you can get it working to merge that would be great. Cheers.
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.
Ah, yeah, I see that now. I'll try to work through that and get the tests working again.
@jonhealy1 @lossyrob I didn't have write permissions to this branch, so I opened up a separate PR (#455). |
Resolves: #333
Resolves: #334
Related Issue(s): #333 #334
Description:
This fixes how version 1.0.0-rc.1 extensions are handled because they are still using the shortened identifiers. It also fixes the schemas for 1.0.0-beta.1 because these are not on schemas.stacspec.org.
PR Checklist:
scripts/format
)scripts/test
) - I need a password for this?