Mistaking platform and instrument groups

[graemenott]

When attempting to run release with spif_extended_example.yaml, vocal attempts to apply instrument group attr requirements to platform group. Results in error below.

Error in dataset: /home/graeme/git/spif-std/standard/v1/definitions/spif_extended_example.yaml
root -> groups -> platform -> attributes -> instrument_name: Field required
root -> groups -> platform -> attributes -> instrument_long_name: Field required
root -> groups -> platform -> platform: Field required
Error: Failed to create versioned products

Applying InstrumentGroupAttributes() to all groups?

[davesproson]

@graemenott Indeed. I'm not sure the platform group was a thing that was considered when this was first put together. Do we have a full spec of what a spif file should look like anywhere. At the moment there's definitely the requirement that a top-level group is an instrument group.

First thing to define is exactly what a platform group needs to look like...

[graemenott]

Any other group, including platform, is an optional. So it vocal applying the instr group validators to any group in the root? Is it possible to make it so any other group that isn't an instrument group validated against the definitions only?

Is it becoming a right pain that the instrument group does not have a defined or at least known name? Do we need to make a mandatory global attribute that is the instrument group name/s?

Isn't the full spif spec in the spif_example as it has all the mandatories?

[davesproson]

The difficulty is that if an instrument group is a subset of a. n. other group (which I believe it is), and you allow either Group or InstrumentGroup to exist on the dataset, then an invalid instrument group (say there's a bit of missing metadata, or a typo etc.) then this will just be mapped to a generic group.

Isn't the full spif spec in the spif_example as it has all the mandatories

full spec != mandatories, as it may---as in this case---exclude optionals.

[davesproson]

A way to get around this would be to have something like a group_type attribute, which could be either instrument, platform or other, and as long as these are defined as literals, we can be sure things get parsed to the correct models.

[davesproson]

Just so I've got this straight in my head:

• root can have instrument groups, an (optional?) platform group and any other generic groups we don't care about
• instrument groups must have a core group, and may have other groups we don't care about
• core groups have a bunch of requirements on variables/dimensions. May have generic subgroups we don't care about
• platform groups currently have no stipulations about anything. May have other generic sub groups.

??

[graemenott]

Sorry, prep'ing for volcano. Yes to your last. We only came up with platform as it seemed a reasonable name, it is a sensible inclusion but I saw it as just another optional group in the root. To summarise?

• There must be at least one instrument group in the root
  • In the instrument group there are some mandatory attrs and vars
  • There must be a core group in the/each instrument group
    • In each instrument/core there are mandatory vars but no mandatory attrs
    • There are no restrictions on optional groups or anything in the instrument/core (this is possibly not great as may lead to confusion but probably easier/fine)
  • There are no restrictions on additional groups or anything in the instrument group/s
• There are no restrictions on additional groups or anything in root

[davesproson]

Ah yes, the biannual volcano panic.

Can chat about this tomorrow if you're in. I think the commit associated with this issue 'solves' the problem, but with greater flexibility comes less specific/more verbose error management.

[graemenott]

A way to get around this would be to have something like a group_type attribute, which could be either instrument, platform or other, and as long as these are defined as literals, we can be sure things get parsed to the correct models.

So that would be a group attribute in all the groups one down from root? I'm trying to think from the point of view of an arbitrary user who is not using vocal. They need to distinguish between the groups that have the raw image data in and those that are something else entirely? Which would be easier, a comma-delineated string attr in the root or an attr in each group? One you need to walk and one you need to parse so neither are ideal.

Oh, "instrument" is probably not a great tag in this context; "image", "raw_image", ?? ...I'm beginning to hate this.

[graemenott]

So latest idea...is to do both.

1. Mandatory root attribute called imager_groups which is a comma- or space-delineated string of imaging instrument group names.

   This attribute is to allow users to find the group/s that contains the image data amongst other non-image groups in the root.

2. Mandatory group attribute group_type in all groups except the root which must be one of;

   • "imager"
   • "core"
   • "platform"
   • "other"

   These strings shall be used to validate SPIF nc files.

See issue #5 about changing "instrument" to "imager".

[graemenott]

1. Should probably make root attribute imager_groups recommended as opposed to mandatory so that is not required when file only contains a single imager group. However is required for now for simplicity.
2. Done.

[davesproson]

Reposting with correct nomenclature for clarity...

• There must be at least one imager group in the root

  • In the imager group there are some mandatory attrs and vars

  • There must be a core group in the/each imager group

    • In each imager/core there are mandatory vars but no mandatory attrs
    • There are no restrictions on optional groups or anything in the imager/core (this is possibly not great as may lead to confusion but probably easier/fine)

  • There are no restrictions on additional groups or anything in the imager group/s

• There are no restrictions on additional groups or anything in root

[davesproson]

Does this look about right relationship-wise?

[graemenott]

Reposting with correct nomenclature for clarity...

* There are no restrictions on optional groups or anything in the `imager/core` (this is possibly not great as may lead to confusion but probably easier/fine)

Good point about this. I agree that it probably shouldn't be allowed to have sub groups of imager/core but let's leave that for the moment/forever.

Just give an hour or two to work out what's happening in this diagram!