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

Bands RFC #1213 #1254

Merged
merged 12 commits into from
Aug 5, 2024
Merged

Bands RFC #1213 #1254

merged 12 commits into from
Aug 5, 2024

Conversation

m-mohr
Copy link
Collaborator

@m-mohr m-mohr commented Sep 27, 2023

Related Issue(s): #1213

Proposed Changes:

  • bands is a new field in common metadata to replace eo:bands and raster:bands (#1213)
  • The fields data_type, nodata, statistics and unit have been added to common metadata (#1213)

This PR needs to wait for the EO and Raster Extension PRs.

ToDo:

PR Checklist:

  • This PR is made against the dev branch (all proposed changes except releases should be against dev, not master).
  • This PR has no breaking changes.
  • I have added my changes to the CHANGELOG
    or a CHANGELOG entry is not required.
  • This PR affects the STAC API spec,
    and I have opened issue/PR #XXX to track the change.

@emmanuelmathot
Copy link
Collaborator

Concerning the last commit, there is no way to make suggestions on unchanged file so I simply committed it. Let me know.

@m-mohr
Copy link
Collaborator Author

m-mohr commented Dec 5, 2023

We should discuss whether we want to explicitly disallow to use of "bands" for single-band use-cases where instead people must provide the properties at the asset/item properties level directly.

@emmanuelmathot
Copy link
Collaborator

We should discuss whether we want to explicitly disallow to use of "bands" for single-band use-cases where instead people must provide the properties at the asset/item properties level directly.

I vote +1 for disallowing unique band

@emmanuelmathot emmanuelmathot added this to the 1.1 milestone May 14, 2024
item-spec/common-metadata.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@emmanuelmathot emmanuelmathot left a comment

Choose a reason for hiding this comment

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

@m-mohr Are we good for a merge in dev? At least to be able to release a beta version to validate extensions and pending PR that requires the band construct?

@m-mohr
Copy link
Collaborator Author

m-mohr commented Jul 8, 2024

The extensions should validate without requiring 1.1.

I think we could do the following:

  • Finish and merge extension PRs (potentially changing the base branch temporarily to the latest stable version to not confuse readers) - started this for eo
  • Release rc.1 versions
  • Update and merge this PR
  • Release 1.1.0-rc.1 version of STAC
  • Prepare everything for 1.1.0 (especially examples)
  • Let users check everything for a month or two
  • Release new stable versions for everything
  • Change the base branch to main again

stac-extensions/raster#45 should finally be ready for review!

@m-mohr
Copy link
Collaborator Author

m-mohr commented Jul 8, 2024

We should probably discuss in the PR when properties should be used on the asset level and when people should have a single-element bands array. I assume it's just if it's a named entity (i.e. name is required)?

@emmanuelmathot
Copy link
Collaborator

you mean allowing single band to be able to set properties only at band level?

@m-mohr
Copy link
Collaborator Author

m-mohr commented Jul 9, 2024

I meant guiding users between the different options.

Single Band

For a single band asset, you could for example do one of the two options:

(1)

{
  "assets": {
    "example": {
      "href": "example.tif",
      "bands": [
        {
          "data_type": "uint16",
          "eo:common_name": "red",
          "raster:spatial_resoltion": 10
        }
      ]
    }
  }
}

or

(2)

{
  "assets": {
    "example": {
      "href": "example.tif",
      "data_type": "uint16",
      "eo:common_name": "red",
      "raster:spatial_resoltion": 10
    }
  }
}

If you add a name, it get's even more unclear:

(3)

{
  "assets": {
    "example": {
      "href": "example.tif",
      "bands": [
        {
          "name": "r",
          "data_type": "uint16",
          "eo:common_name": "red",
          "raster:spatial_resoltion": 10
        }
      ]
    }
  }
}

Would you then do the following?

(4)

{
  "assets": {
    "example": {
      "href": "example.tif",
      "data_type": "uint16",
      "eo:common_name": "red",
      "raster:spatial_resoltion": 10,
      "bands": [
        {
          "name": "r"
        }
      ]
    }
  }
}

Multi-band

And then for multi-band files, you can often also de-duplicate some properties.

Old example from 1.0:
(5)

{
  "assets": {
    "example": {
      "href": "example.tif",
      "eo:bands": [
        {
          "name": "r",
          "common_name": "red"
        },
        {
          "name": "g",
          "common_name": "green"
        },
        {
          "name": "b",
          "common_name": "blue"
        }
      ],
      "raster:bands": [
        {
          "data_type": "uint16",
          "spatial_resolution": 10
        },
        {
          "data_type": "uint16",
          "spatial_resolution": 10
        },
        {
          "data_type": "uint16",
          "spatial_resolution": 10
        }
      ]
    }
  }
}

If you would just merge them, you end up with:
(6)

{
  "assets": {
    "example": {
      "href": "example.tif",
      "bands": [
        {
          "name": "r",
          "eo:common_name": "red",
          "data_type": "uint16",
          "raster:spatial_resolution": 10
        },
        {
          "name": "g",
          "eo:common_name": "green",
          "data_type": "uint16",
          "raster:spatial_resolution": 10
        },
        {
          "name": "b",
          "eo:common_name": "blue",
          "data_type": "uint16",
          "raster:spatial_resolution": 10
        }
      ]
    }
  }
}

But ideally, I think, you could simplify to:
(7)

{
  "assets": {
    "example": {
      "href": "example.tif",
      "data_type": "uint16",
      "raster:spatial_resolution": 10,
      "bands": [
        {
          "name": "r",
          "eo:common_name": "red"
        },
        {
          "name": "g",
          "eo:common_name": "green"
        },
        {
          "name": "b",
          "eo:common_name": "blue"
        }
      ]
    }
  }
}

I think the difference between 5 and 7 shows the beauty of the new concept nicely ;-)

Edit: I tried to write something in best practices: 5b9af3b Please review.

best-practices.md Outdated Show resolved Hide resolved
Co-authored-by: Pete Gadomski <pete.gadomski@gmail.com>
@m-mohr m-mohr merged commit 53fa030 into dev Aug 5, 2024
2 checks passed
@m-mohr m-mohr deleted the bands branch August 5, 2024 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants