Skip to content

Conversation

@m-mohr
Copy link
Contributor

@m-mohr m-mohr commented Oct 20, 2021

Opening this so that I have something to assign me to. I'm wondering whether we @drwelby should do a working session and prepare this more so that it can go into a broader review?

@drwelby
Copy link
Collaborator

drwelby commented Oct 20, 2021

@m-mohr I have time available to do a working session. I also wonder if we can do some kind of FOAF group within the ard.zone schedule next week.

@m-mohr
Copy link
Contributor Author

m-mohr commented Oct 21, 2021

@m-mohr I have time available to do a working session.

Sounds good. Let me know what time would suit you best.

I also wonder if we can do some kind of FOAF group within the ard.zone schedule next week.

Sounds good, too, although I'm only attending for a couple of selected talks. But if you have a time slot, let me know and I'll try to join.

@duckontheweb
Copy link

@drwelby @m-mohr Does this PR represent the most up-to-date work on this extension so far?

@drwelby
Copy link
Collaborator

drwelby commented Feb 15, 2022

@duckontheweb I've noodled on it some more, but if you have any quick feedback on what's in the PR as it stands I'll try to account for it before update.

Copy link

@duckontheweb duckontheweb left a comment

Choose a reason for hiding this comment

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

@drwelby @m-mohr Thanks for working on this, I think it's going to play into the work we want to do on the Label Extension. It seems like there might be some content that still needs to be clarified/added from the "working session" that you both talked about. Is that the case?

README.md Outdated
This is a much more detailed description of the field `template:new_field`...

### XYZ Object
_"values" is shown as a list of any object. I think that could include Range objects for continuous data_

Choose a reason for hiding this comment

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

I think this would be a useful feature to support. Could we add some specific instructions/examples on how to represent these continuous values?

Copy link
Collaborator

Choose a reason for hiding this comment

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

What bothers me is that putting in ranges seems like you have data that isn't "classed" in the first place. Maybe that's too strict though. Do you have an example of how you can use this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you could also give an example of your ML label rasters in #1 (comment) that would help out.

Choose a reason for hiding this comment

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

Yeah, that's a fair point.

I don't have an example at hand, but I know I've worked with land cover rasters in the past where a particular class was represented by a value range for one reason or another. However, they were usually limited to 2-3 values, so we could reasonably handle that using a list.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yup, I've seen LC coverages that have classes that get merged, like "Scrub Woodland", "Deciduous Woodland", etc, and they get flattened to "Woodland". So you're reclassing the data after the fact, and that seems interpretive and not descriptive. Repeating classes in a list doesn't hide the original data intent as much and might convince people to properly class their values?

I mostly want to avoid having data that is really continuous (like a confidence percentage) and someone saying "yeaahhh, 75% confidence sounds good enough for these values to describe a feature"

README.md Outdated
Comment on lines 60 to 62
_@mmohr picked "summary" as distinctive from the "description" field commonly used elsewhere with the expectation that CommonMark would not be used and these descriptions would be short in nature_

_"Summaries" are mentioned elsewhere in the STAC spec (https://github.com/radiantearth/stac-spec/blob/master/collection-spec/collection-spec.md#summaries) to summarize data statistically so maybe "description" is OK_

Choose a reason for hiding this comment

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

It seems like both of these potentially conflict with similarly named fields elsewhere in the spec. "description" makes the most sense to me, but I don't have a strong opinion.

README.md Outdated
Comment on lines 65 to 77
### _Common fields used elsewhere_

| Field Name | Type | Description |
| -------------------- | ------------------------- | ----------- |
| name | string | Name of the class |
| description | string | Description of class |
| title | string | Like "name" but formatted for display|

_"Description" vs "Summary" is addressed above_

_"name" could be useful as a field to identify a class. If classes are mapped to names ((instead of a list)) it would make classes more machine readable. A field named "name" as optional would be useful when classes don't have names._

_"title" in other formats like SLD are useful for generating legends and such, but fall into the category of styling and display and probably should be left to styling files which could be linked as assets._

Choose a reason for hiding this comment

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

It's not clear to me how this fits into the spec. Are these referring to the existing Asset fields? Is this an alternate definition of the Class Object that we are considering?

README.md Outdated
Comment on lines 79 to 85
### _Other fields suggested in discussions_

| Field Name | Type | Description |
| -------------------- | ------------------------- | ----------- |
| color | string | Color to display this class |

_This is another case of using a metadata field for styling and probably should be discouraged._

Choose a reason for hiding this comment

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

I agree that we should avoid styling metadata. Maybe this should be removed from the PR?

Copy link
Collaborator

Choose a reason for hiding this comment

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

README.md Outdated
Comment on lines 24 to 38
## Asset Properties

For single-band rasters

## Raster Band (Raster Extension)

For multiband rasters

## Table Column (Table Extension)

For tabular or vector datasets

## Collection Fields

In `summaries` field

Choose a reason for hiding this comment

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

Is there some content missing here? I'm not quite sure what this is trying to express.

Copy link
Collaborator

Choose a reason for hiding this comment

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

sort of a checklist

README.md Outdated

| Field Name | Type | Description |
| -------------------- | ------------------------- | ----------- |
| values | [Any] | **REQUIRED** Values in the class |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@drwelby
Copy link
Collaborator

drwelby commented Mar 15, 2022

Merging to main since this has enough feedback as an initial draft to be a good starting point.

@drwelby drwelby merged commit e7b7f47 into main Mar 15, 2022
@drwelby drwelby deleted the draft branch March 15, 2022 20:25
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.

4 participants