Placeholder validation skips unit checking

[h-mayorquin]

Speed/# degree/s passes validation with allow_placeholders=True, but
Speed/45.2 degree/s fails because degree/s is not a valid Speed unit.

Check the following script:

from hed import load_schema_version
from hed.models import HedString

schema = load_schema_version("8.4.0")

# Template with placeholder - PASSES (but shouldn't, since degree/s is invalid)
template = HedString("Speed/# degree/s", schema)
issues_template = template.validate(allow_placeholders=True)
assert issues_template == []

# Same template with value substituted - FAILS
expanded = HedString("Speed/45.2 degree/s", schema)
issues_expanded = expanded.validate(allow_placeholders=False)
print(issues_expanded)

Output:

[{'code': 'UNITS_INVALID', 'message': 'Invalid unit - "Speed/45.2 degree/s" valid units are "kph,m-per-s,mph"', 'severity': 1, 'source_tag': <hed.models.hed_tag.HedTag object at 0x731f1fd078f0>}]

I think that the unit error could be caught at template validation time as well, right?

I also tested this behavior here:
https://hedtools.org/hed/strings

If you tell me what repo covers this I can open an issue about it

[VisLab]

HED requires that units conform to the unit classes specified in the schema. The speedUnits for schema 8.4.0 are:

• m-per-s
• mph
• kph

The m-per-s is an SIUnit and a unitSymbol meaning that the leading m can be modified by any of the unit modifiers that are unit symbols -- so mm-per-s is allowed etc. The speedUnit refers specifically to rate of change of length, not rate of change of anything else.

We don't have a generalized mechanism for rate of arbitrary units at this time. One way you can currently express this in HED is (Temporal-rate/#, Label/Degrees-per-second).

If you have some ideas about how to improve the HED schema in this area or would like to initiate a discussion of this issue, we can transfer this issue to the hed-schemas/issues for more work.

[h-mayorquin]

Yes, I am aware that you have don't have units for degrees / s. That's another issue. This issue here is about the validation failing to detect that when there is a placeholder.

We don't have a generalized mechanism for rate of arbitrary units at this time. One way you can currently express this in HED is (Temporal-rate/#, Label/Degrees-per-second).

Thanks for this tip.

[VisLab]

Yes you are right -- it probably should be detected at the placeholder stage, but we'll have to go over it carefully because there are certain things that we have to hold off validating until assembly -- for example the value class validation. We will try to address one way or the other before the next release.

You don't have to post another issue for hedtools. It relies on the python hedtools. When we do a hedtools release, we update the required version in hed-web and redeploy the docker to our server.

[h-mayorquin]

Thank you.