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.

Sign up for GitHub

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

Jump to bottom

fix: STAC files should comply to 1.0.0-beta.2 of the specification #1176

Merged
merged 1 commit into from
Sep 22, 2020
Merged

fix: STAC files should comply to 1.0.0-beta.2 of the specification #1176

merged 1 commit into from
Sep 22, 2020

Conversation

jacott
Copy link
Contributor

@jacott jacott commented Sep 15, 2020

  • Add linz basemaps schema.json
  • Fix extent/spatial/bbox to number[][]
  • description is always required
  • Set stac_version to 1.0.0-beta.2
  • Don't use projection extension for collection.json

@jacott jacott requested a review from blacha as a code owner September 15, 2020 04:22
blacha
blacha reviewed Sep 15, 2020
View reviewed changes
packages/cli/src/cog/cog.stac.job.ts
metadata.bounds.map((a) => Bounds.fromJson(a)).reduce((sum, a) => sum.union(a)),
);
const bbox = [
sourceProj.boundsToWgs84BoundingBox(
Copy link
Member

Choose a reason for hiding this comment

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

Does this break with the antimeridian?

is the reason their are multiple bbox's for the antimeridian?

Copy link
Contributor Author

@jacott jacott Sep 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

No. only one bbox is needed even when AM is crossed; it just means the first longitude will be greater than the second longitude

Copy link

Choose a reason for hiding this comment

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

Hi Guys, it's best practise to split across the anti-meridian for interoperability reasons with clients. See https://tools.ietf.org/html/rfc7946#section-3.1.9

Copy link
Member

Choose a reason for hiding this comment

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

@jacott with Jeremy's comment maybe we should split it on the antimeridian? Does the stac spec tell us not to?

Copy link
Contributor Author

@jacott jacott Sep 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@blacha , @palmerj The GeoJSON RFC does not split bbox https://tools.ietf.org/html/rfc7946#section-5.2. STAC uses the GeoJSON for items but I defer to others and will split if you think I should.

Copy link
Contributor Author

@jacott jacott Sep 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

What Jeremy's link above is for is polygons which we do split.

Copy link

Choose a reason for hiding this comment

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

Ok thanks. You are correct. As long as we split the footprint geometry then we are ok. Clients querying on the bbox via the STAC API will of course need to normalise their input bboxes

@jacott jacott marked this pull request as draft September 15, 2020 23:40
@jacott jacott force-pushed the fix/stac-compliance branch 3 times, most recently from c7df266 to 1d9ce32 Compare September 16, 2020 23:39
@jacott jacott marked this pull request as ready for review September 16, 2020 23:39
@jacott jacott marked this pull request as draft September 16, 2020 23:40
@jacott jacott marked this pull request as ready for review September 17, 2020 00:08
@jacott jacott force-pushed the fix/stac-compliance branch 2 times, most recently from cf432d8 to ac3c3da Compare September 17, 2020 21:12
@jacott jacott requested a review from blacha September 21, 2020 02:18
blacha
blacha reviewed Sep 21, 2020
View reviewed changes
packages/landing/scripts/deploy.js
/**
* Upload all files from the dist directory, including any sub directories, to S3 bucket .
*/
const uploadDirectory = async () => {
Copy link
Member

Choose a reason for hiding this comment

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

NIT: Why make a function and call it directly after? could you not just execute the function

@kodiakhq kodiakhq bot merged commit d2fe323 into master Sep 22, 2020
@kodiakhq kodiakhq bot deleted the fix/stac-compliance branch September 22, 2020 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@palmerj palmerj palmerj left review comments

@blacha blacha blacha approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jacott @palmerj @blacha