Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SignalMetadata units attribute is too broad #616

Open
DiamondJoseph opened this issue Oct 18, 2024 · 1 comment
Open

SignalMetadata units attribute is too broad #616

DiamondJoseph opened this issue Oct 18, 2024 · 1 comment

Comments

@DiamondJoseph
Copy link
Contributor

SignalMetadata, which is used by SoftSignals to present information about units and precision, uses just a str as its datatype.
While the use case of providing arbitrary configuration-like metadata is to be discouraged, the use of consolidating several signals into another value should get additional support for developers to ensure data is consistent and maintains integrity.

This could be accomplished by ensuring that the units is an instance of e.g. this library's SI units representation. This would allow propagating units from the physical signals that back the SoftSignal, including relations between multiple signals in a simple way.

Acceptance Criteria

  • SignalMetadata.units are better supported and validated
@Tom-Willemsen
Copy link
Member

If we do this, could we explore making the underlying units library pluggable? e.g. at ISIS we're already using scipp for our units handling (which uses llnl-units under the hood), I could imagine other facilities wanting to use pint or forallpeople as you suggested, or various domain-specific libraries, or ...

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

No branches or pull requests

2 participants