Implementation detail: To keep things simple, I have reimplemented Quantity's__gt__ operator as(self - other).magnitude > 0. If this is undesirable because of corner cases with unsigned integer dtypes, I am happy to rework it.

Thank you for this, the new behaviour as you describe it does seem to be an improvement. There is, however, a risk that it breaks downstream code that relies on the current behaviour, so I would like to test it first with some of the downstream projects, such as Neo.

I note that this would require#248 to be merged first.@wagenadl perhaps you could modify the implementation, at least temporarily, to not use the.plain property, while we discuss that in parallel?

In addition, please could you add some unit tests, for the "standard" usage, and for any potential corner cases you can foresee?

I agree that this patch may break some extant code. It is, of course, not up to me to decide whether that is an acceptable cost for the improvement.

To reiterate why I think the new behavior is preferable: Both versions of the code agree thatpq.ms == pq.s is false, which makes sense because 1 ms ≠ 1 s. However, under the old behavior, bothpq.ms == 1 andpq.s == 1 were true. This violated transitivity (if A==B and B==C then A==C). Under the new behavior,pq.ms == 1 andpq.s == 1 are both false. Likewise, under the old behavior,pq.s * pq.kHz == 1 was true, even though manifestly 1 s × 1 kHz = 1000 ≠ 1.

In my latest commit, the "test_comparison.py" test file now verifies that the behavior of the comparison operators matches the new specification. This involved changing various assertions and adding some new ones.

Also, this patch now also works independently of the recently merged “dimensionless_magnitude” property.