MetrologyNamespace (initial PR)

[andrewgsavage]

transferred from quantity-dev/quantity-api#5

[nstarman]

Looks really good!
Minor comments about the docstring formatting.

[jorenham]

I notice that there are several type parameters with generic upper bounds (basically everything that uses TypeVar). As I mentioned previously, this is unfortunately not supported in Python.

I'd recommend adding type-checkers to the CI, so that you can avoid this in the future.

[lucascolley]

@jorenham yes, we have them, you can observe in CI the errors I pointed out at #9 (comment). My question is how to address those errors.

[lucascolley]

In other words, do we have to use just Quantity[Any, Any, Any], or is there a better solution? The hints from basedmypy seem to be suggesting that there is some sort of alternative.

[jorenham]

@jorenham yes, we have them, you can observe in CI the errors I pointed out at #9 (comment). My question is how to address those errors.

Ah nice. I must've missed it because I did not expect type-checkers to run under the "lint" label, but it's of course true that type-checking is a form of linting.

In other words, do we have to use just Quantity[Any, Any, Any], or is there a better solution? The hints from basedmypy seem to be suggesting that there is some sort of alternative.

Basedmypy is indeed the only one that supports it. But it requires that you use all off the type-parameters in the upper bound on the right-hand-side. Put differently; it doesn't accept free type-parameters. That's because free type-parameters don't do anything, so you might as well leave them out (and use Any instead).

However, by relying on this exclusive basedmypy feature, you're effectively requiring all of your users to only use basedmypy. Because, after all, the other type-checkers don't support these parametrized upper bounds of type-parameter.
So if you want to also support e.g. vanilla mypy, vanilla pyright, or basedpyright, then you shouldn't use this feature. I suppose you could consider it backwards-incompatible, in that sense.

The typing-spec compliant alternative is precisely like you said — using Any as type-argument for all generic upper bounds. Depending on the situation you could, of course, also choose something narrower than Any. But I'm afraid that you can't do much better than that — even optype isn't be able to massage this reality.

[lucascolley]

The typing-spec compliant alternative is precisely like you said — using Any as type-argument for all generic upper bounds. Depending on the situation you could, of course, also choose something narrower than Any. But I'm afraid that you can't do much better than that — even optype isn't be able to massage this reality.

Okay, thanks! How does b70a68b look?

[jorenham]

maybe I'm missing something, but I see only two type parameters here:

metrology-apis/src/metrology_apis/__init__.py

Line 35 in 05af87c

class Quantity[V, U: Unit](Protocol):

Nitpick: Since you're using Quantity[Any, Any(, Any)?] a couple of times now, you could consider defining a type _AnyQuantity = ... for the the sake of DRY.

[lucascolley]

I think you're looking at a different commit to the tip of this branch

[mhvk]

Further to the above, maybe some relevant points (though likely for v2!):

• A general function like convert(unit1, unit2, value) in the end needs the units to tell how to do the conversion (given, e.g., logarithmic units like decibel). I think that is absolutely fine, but possibly makes the case for a proscribed method on the unit slightly stronger (though, on balance, I think we should leave it as an implementation detail).
• Missing in our discussion so far is the need for a general function that tells for a given operation/function, (1) how to convert the input arguments, (2) what the result unit is. I.e., for addition: arg1: no conversion, arg2, convert to unit1, result_unit: unit1; for sin: convert to rad, result dimensionless. Do we need to define an API for this, or is it an implementation detail? One could envision something analogous to result_dtype in the array API (although here it needs the function and other of the arguments matters), say result_unit(function_name: str, units: tuple[U, ...]) -> tuple[result_unit: U, tuple[callable, ...]] EDIT: more logical would be to return units that the arguments have to be convered into (instead of the callable).

[phlptp]

Is this for C to F, etc.? My sense would be that such things need a different unit class that know how what their offset is. In >astropy, we actually never got around to dealing with this properly (since thankfully almost nobody uses C or F), but we do >something similar for magnitudes, decibels and other logarithmic units. It is perhaps relevant, since that is one argument for >carrying the conversion method on the unit rather than as a function (maybe better post separately below).

one of the key requirements for the library was to support power systems units, which look like per-unit MW or per-unit V. These are essentially dimensionless units but it is necessary to track the original unit as well so to know what it can legitimately be converted back to. In the computation this allows transformers to be mostly ignored in some computations. But we need a mechanism to get back to the original values as well. Hence the use of dedicated per-unit flag in the unit definition. I wouldn't really suggest trying to make this mechanism more broadly applicable as it is a rather niche use case, though we do also use for other things like strain for example and other types of ratio measurements.

For C to F and R and a few other temperature units there is a separate chunk of code that knows about those and do the appropriate conversion. There is also an equation flag and overloaded bit codes that know of different types of operations and inverse operations for things like dB and bel, and a few other logarithmic type operations that are inherent in some units. There isn't a general datum shift mechanism though.

[mhvk]

Interesting. That sounds not dissimilar to our logarithmic units, where we also have to keep track of the reference unit (e.g., dBm = Decibel(mW)). It is definitely tricky to get those things right.

[nstarman]

@mhvk

I like the idea but the names not as much 😺
Note that this being on the metrology namespace, it is OK to have a more common name ("Namespaces are one honking great idea" and all that). Maybe convert(from_unit, to_unit, value) and to_unit(q, new_unit)?

Namespace collision isn't just between namespaces, it's within! If we implement Quantity Kinds then it will be necessary to support conversion between kinds.
If we're quoting Python zen 😆 then "Explicit is better than implicit". convert what? If convert were a method on Unit then it's more explicit. But as a function, it's not.

I think that is absolutely fine, but possibly makes the case for a proscribed method on the unit slightly stronger (though, on balance, I think we should leave it as an implementation detail)

I think this is an important discussion. Function or method?

Aside, astropy has the concept of equivalencies (treating, e.g., radian as dimensionless, or "arcsec" as equivalent to "parsec" (just do the inverse), or temperature as equivalent to energy (multiply with k_B). Definitely not something one should insist on, obviously, but something not to preclude either, i.e., we may want to leave **kwargs?

Yes. This is definitely something to discuss. Even at the level of which functions should have **kwargs in their signatures? There are other functions, like asquantity which we imagine implementing libraries will want to extend further.

[lucascolley]

which functions should have **kwargs in their signatures

I think we should follow the array API standard's lead https://data-apis.org/array-api/draft/purpose_and_scope.html#conformance:

A conforming implementation of the array API standard may provide additional features (e.g., values, objects, properties, data types, functions, and function arguments) beyond those described in this specification.

thus **kwargs does not appear in the standard.

[nstarman]

I have mixed feelings on their stance. It makes for easier-to-read docs, but it's actually less correct from a static typing perspective. The Array API doesn't actually have statically typed reference code, only the docs, so our situation is a bit different.

[andrewgsavage]

We've hit an impasse. This feels close to merging - should we leave the unit conversion to another PR?

[nstarman]

I think so! Things that seem to need further discussion:

1. unit conversion method / function names
2. adding (or not) kwargs to method signatures
3. [other stuff?]

[lucascolley]

one question @andrewgsavage !

[andrewgsavage]

any idea why the lint CI is falling over now?

[lucascolley]

prefix-dev/pixi-build-backends#130

I can push a workaround if they don't respond today

[lucascolley]

thanks all, looks good from my side!

[mhvk]

I'm OK with getting this in, but note that much seems implicitly defined by typing. I'm not at all an expert on this, but there are oddities:

1. What does Quantity[V, U, D] mean? Is it now required for Quantity to be (partially) defined by a Dimension? Or is that still optional. I can see the point of Dimension, but astropy's Quantity makes clear one can get a nice working system without it.
2. I don't understand the typing for the operators: the typing suggests that other is required to be a Quantity, but surely that is incorrect. At least, I'd assume we want q * 5 to work, and probably dimensionless_q + 10. as well).

I guess the second option brings up an overall worry with most PRs so far: we've jumped straight into abstractions, but I at least work by far the best with concrete examples. Comments and docstrings would seem appropriate!

[lucascolley]

I'd assume we want q * 5 to work

Yes, perhaps we want to expand

self: "Quantity[V, U, D]", other: "Quantity[op.CanRMul[V, R], U, D]"

to

self: "Quantity[V, U, D]", other: "Quantity[op.CanRMul[V, R], U, D] | op.CanRMul[V, R]"

[nstarman]

What does Quantity[V, U, D] mean? Is it now required for Quantity to be (partially) defined by a Dimension? Or is that still optional. I can see the point of Dimension, but astropy's Quantity makes clear one can get a nice working system without it.

To clarify, what this means is that Q is defined not by the value of the dimension, but by the type. In principle one could build a unit library that allows for many different dimension types (from the same or different libraries). Then one could build a Quantity type on top of that supports many different unit base classes (from the same or different libraries). This is a flexible definition. The optimal thing would be

class Q[V, U[D]]: ...

In which case the "D" can be "hidden" / more easily left unspecified.
However Python does not (yet/ever) permit this "higher"-kinded typing so we must flatten the type.

[mhvk]

@lucascolley - seems sensible for multiplication. What about the example I gave for addition? Here, unfortunately, it does depend on the unit instance, not the type... I guess the question boils down to whether, generically, any V is treated as equivalent to a dimensionless quantity.

Note that in astropy there is an exception to this: we special-case zero as equivalent to 0 with any unit, so that q < 0 always works (same for inf and -inf). Is this even expressible in typing?

Note that perhaps both the above should be restricted - I've been arguing elsewhere for not pre-allowing flexibility or baking in complexity. So, if some other unit libraries do not support adding non-quantities to dimensionless ones, or do not allow comparing with 0, then that's fine. Ideally, though, the documents that we produce explicitly state that support for examples like the ones above is not required.

p.s. Perhaps, though, we should merge this and just open an issue about details like these...

[nstarman]

Agreed. Generalizing non-Quantities to Quantities is challenging and may be best left as a choice for implementing libraries. The common API of course is for Q2Q ops to work!

[andrewgsavage]

alright, merging this then we can move the last comments to their own issues.