search

pds-data-dictionaries / PDS4-LDD-Issue-Repo

Issue repository for tracking all PDS4 Discipline Dictionary-related issues, new feature requests, and releases.
Apache License 2.0
2 stars 1 forks source link

[ldd-img] Add `Units_of_Misc` to `img:Sampling/img:*track_summing` attributes to allow `unit="pixel"` #280

Closed cgobat closed 3 months ago

cgobat commented 5 months ago

Issue Type

ENH

Describe the issue identified (if applicable)

The img:crosstrack_summing and img:downtrack_summing attributes are described as

the number of detector pixel values in the [cross/down]track direction that have been averaged to produce the final output pixel.

Since the values contained here represent a number of pixels, the attributes should have unit="pixel" in labels. However, these two attributes currently have Units_of_None, which does not allow this.

Describe the solution you'd like

Change Units_of_None to Units_of_Misc in the DD_Attribute definitions, and possibly add <specified_unit_id>pixel</specified_unit_id> as well.

Describe alternatives you've considered

Leave as-is and omit units.

LDD Dictionary Version

Current (1.8.9.0 as of writing)

PDS4 IM Version

All

Need-by Date

~next month, if we're going to make use of this change

Additional context

Ref https://github.com/pds-data-dictionaries/ldd-nh/issues/16#issuecomment-2050350766

thareUSGS commented 5 months ago

dev version built for review: https://github.com/pds-data-dictionaries/ldd-img/tree/v1900/build/development

Haven't really thought too much about this -- but does changing Units_of_None to Units_of_Misc make it non-backwards compatible. I don't think it does. thoughts? @cgobat

cgobat commented 5 months ago

Hmm. I think this technically is a non-backwards-compatible change. Maybe the attribute versions should get bumped to 2.0 instead of just 1.1? That might be sufficient.

thareUSGS commented 4 months ago

@cgobat I am ready to push this into main unless you have anything else. For IMG, there was some emails for Classes like Data_Processing to be repeatable. Currently, if that is needed, I think "Imaging" is actually repeated.

But since that is backwards compatible, I would be fine letting Data_Processing be repeatable if you think that might help.

Also, as much as I want to change sequence_number to an INT, I'm not sure why or if a string was previously used or needed. SO I will likely keep it as a string for now.

cgobat commented 4 months ago

I don't have a need for Data_Processing (or even Imaging as a whole, for that matter) to be repeatable. I think Anne was just trying to understand the intent behind some of the attributes when she was asking about class repeatability.

I am good to go on this units change. Your reasoning about sequence_number staying a string makes sense to me.

cgobat commented 3 months ago

Resolved via pds-data-dictionaries/ldd-img#130