Skip to content

Python: never write the FIT invalid value as a measurement. - #46

Open
sanderroosendaal wants to merge 2 commits into
mainfrom
python/invalid-values
Open

sanderroosendaal wants to merge 2 commits into
mainfrom
python/invalid-values

Conversation

@sanderroosendaal

Copy link
Copy Markdown
Collaborator

Implements the Python side of #38 (spec §7.2).

  • encode clips a value that would encode to the base type's invalid value (255, 65535, 32767, and the 32-bit equivalents) to the next valid one. Values beyond the type limit still raise CodecError.
  • decode returns None for the invalid value, so the reader treats it as missing.
  • StrokeWork at 65535 now encodes as 65534; a raw 65535 in a file reads as missing.

Note for #34: AverageBoatSpeed at its maximum of 655.35 m/s encodes to 65535 at scale 100 and is clipped to 65534.

🤖 Generated with Claude Code

encode clips a value that would encode to the base type's invalid value to the
next valid one, and decode returns None for the invalid value. Values beyond
the type limit still raise.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
)
invalid = BASE_TYPE_INVALID[field.base_type]
if raw == invalid:
return invalid - 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So we're altering data, doesn't sound like a best practice. I would throw an CodecError as it simply doesn't fit the codec.

Also while we're on this topic, when physical is None shouldn't it return BASE_TYPE_INVALID[field.base_type]?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

On clipping: the merged spec (#38, §7.2) asks for exactly this. Producers MUST NOT write the invalid value for a measurement, and "a measurement that would encode to it is clipped to the next valid value". A value beyond the type limit still raises CodecError; only the single value that collides with "no data" is clipped, by one raw step. If we would rather raise there, that is a spec change, so it should be raised as an issue against §7.2 rather than decided in this PR.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

On None: encode(None) returning None means "omit the field", which is what §7.2 asks for ("Missing or unavailable fields SHOULD be omitted from the file"). Writing the invalid value instead would also be legal FIT, but it would write a field the spec says to leave out, and it would change how the writer builds records. decode is the symmetric side: it maps the invalid value back to None, so a file from a producer that does write it reads as missing.

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