floordiv (and divmod, modulo, math.floor) raise DimensionalityError

[westurner]

floordiv (and divmod, modulo, math.floor) raise DimensionalityError, but I don't understand why because division by a dimensionless integer does not raise DimensionalityError?

import pint
ureg = pint.UnitRegistry()

x = 10 * ureg.inches
n, remainder = divmod(x, 3)

Raises DimensionalityError: Cannot convert from 'dimensionless' (dimensionless) to 'inch' ([length]), which is what the tests say it should raise:

pint/pint/testsuite/test_quantity.py

Lines 715 to 720 in 4c3114c

	def _test_quantity_floordiv(self, unit, func):
	a = self.Q_("10*meter")
	b = self.Q_("3*second")
	self.assertRaises(DimensionalityError, op.floordiv, a, b)
	self.assertRaises(DimensionalityError, op.floordiv, 3, b)
	self.assertRaises(DimensionalityError, op.floordiv, a, 3)

I could try and redefine floordiv and divmod to just use division and floor? But why is that necessary? That doesn't work either?

import math
def floordiv(x, y):
    n = x / y  # this works just fine
    n_floor = math.floor(n)  # this raises DimensionalityError?
    return n_floor
floordiv(10*ureg.inches, 3)

(10*ureg.inches) % 3 # raises DimensionalityError

[hgrecco]

You are correct. Please submit a PR

[westurner]

Got it. I'm not sure when I will have time for this. In the meantumey, is there a workaround? I tried dividing by ureg(3) and Quantity(3) but didn't have any luck.

…

On Sat, Dec 21, 2019, 12:21 AM Hernan Grecco ***@***.***> wrote: You are correct. Please submit a PR — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#943?email_source=notifications&email_token=AAAMNS3MPWIOZEX5OGRBLSTQZWRXNA5CNFSM4J6AO652YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHOVODQ#issuecomment-568153870>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAMNS6UGQMZMDHQL4NYXOTQZWRXNANCNFSM4J6AO65Q> .

[hgrecco]

I am phasing this to 0.11

[tecki]

Good that I came here before the PR got merged....

This issue is blatantly wrong. Floordiv should absolutetly give a DimensionalityError, because it is a dimensionality error. Your example shows this very well. You want to floor-divide 10" by 3. What in world should the result be? Probably you say 3". But this is exactly the dimensionality error that pint tries heavily to prevent you from. Why? Because I guess we agree that 10" = 254 mm. If you floor-divide that by 3 in your logic, you get 84 mm, which is certainly not the 3" you got from before.

You could argue now so what, apparently floor division does not make sense for united values. But also this is not true. It makes a lot of sense: imagine you have a log of 1 m, and want to cut it in 1" pieces. Then you calculate 1 m // 1 " and voilà, you get a nice 39, which is true whatever unit you convert it to, simply because it actually corresponds to a real-world problem.

All the same applies for modulo. In the end, we should fulfill the Python documentation (see https://docs.python.org/3/library/functions.html#divmod).

Let q = a // b:

In any case q * b + a % b is very close to a, if a % b is non-zero it has the same sign as b, and 0 <= abs(a % b) < abs(b).

This quote is also well-defined if there are units involved. And the current implementation does fulfill this rule (hopefully there are no bugs...)

[westurner]

There should be a way to divmod(10*u.inches, 3) and/or divmod(10*u.inches, 3*u.inches).

Could that be done in a context manager that sets e.g. a thread local flag to not raise because the user is explicitly asking to modulo and preserve the unit?

[westurner]

Use case: I have 10ft of 2x4; how many 4ft sections can I make out of that?

Could you show how to drop the unit, perform modulo division, and re-apply the unit?

[tecki]

Your use case is exactly the use case that works (or at least should work, provided there is no bug).
divmod(10*u.ft, 4*u.ft) should work seamlessly, simply because it makes sense.

On my machine, latest pint:

>>> divmod(10*u.ft, 4*u.ft)
(<Quantity(2, 'dimensionless')>, <Quantity(2, 'foot')>)

that says: if you have a log of 10 ft length and want to chop 4 ft pieces, you get 2 pieces and are left over with 2 ft of wood. In my example above I even do that with mixed units, like this:

>>> divmod(1*u.m, 1*u.ft)
(<Quantity(3.0, 'dimensionless')>, <Quantity(0.08560000000000012, 'meter')>)

meaning: if you have a log of 1 m length and want to chop it into footlongs, you get 3 foot-long pieces and 85.6 mm left over.

I have no idea what the result of divmod(10*u.inches, 3) should be. Just to make sure, whatever the result is, it should be the same as divmod(254*u.mm, 3)

[deeplook]

What is the status of this issue which I discovered when I was about to open a similar issue, but maybe it better fits as a comment here.

from pint import UnitRegistry

ureg = UnitRegistry()
parse = ureg.parse_expression

# The following should both rather return 1 meter, but return 10 meter.

print(parse("5 m % 2"))
print(parse("(5 % 2) m"))

# The following should both rather return 2 meter, but return 10 meter.

print(parse("5 m // 2"))
print(parse("(5 // 2) m"))

# The following raise a somewhat misleading errors:

# 5 * ureg.m // 2
# DimensionalityError: Cannot convert from 'meter' to 'dimensionless'

# 5 * ureg.m % 2
# DimensionalityError: Cannot convert from 'dimensionless' (dimensionless) to 'meter' ([length])

[jules-ch]

Thanks for the feedback @deeplook.

I can reproduce, it seems binary opertors for floordiv & mod operators are missing in the token evaluator.
Evaluation kept going & made implicit multiplication hence the 10 meters result. Might be a good idea to raise Error if token is not mapped to operations.

After adding them we get the proper results.

>>> from pint import UnitRegistry
>>> 
>>> ureg = UnitRegistry()
>>> parse = ureg.parse_expression
>>> parse("5 m % 2")
pint.errors.DimensionalityError: Cannot convert from 'dimensionless' (dimensionless) to 'meter' ([length])
>>> parse("(5 % 2) m")
<Quantity(1, 'meter')>
>>> parse("5 m // 2")
pint.errors.DimensionalityError: Cannot convert from 'meter' to 'dimensionless'
>>> parse("(5 // 2) m")
<Quantity(2, 'meter')>

I'll make a PR for this.

[tecki]

While I am not sure about the missing operators, I can only say that in the example that you give a DimensionalityError is exactly the right thing. What should the result of parse("5 m % 2") be? Just to make sure, it better be the same result as parse("5000 mm % 2"). parse("5 m % 2 m") however should work, simply because it makes sense: if I have a log of 5 m and chop it into 2 m pieces, I have 2 pieces (that's the floordiv) and 1 m left over (that's the modulo).

In short: for floordiv and modulo both operands need to have the same dimensions, like for addition or subtraction, otherwise it makes no sense.

[jules-ch]

It works as expected with the new version. #1471

>>> parse("5 m % (2 m)") 
<Quantity(1, 'meter')>
>>> parse("5 m // (2 m)") 
<Quantity(2, 'dimensionless')>

It only works with the parentheses around 2 m though. Don't know if thats a side effect of the parser.

parse("5 m % 2") should result to Dimensionality error since pint has no idea how to divide your length without unit information.

[deeplook]

While I am not sure about the missing operators, I can only say that in the example that you give a DimensionalityError is exactly the right thing. What should the result of parse("5 m % 2") be? Just to make sure, it better be the same result as parse("5000 mm % 2"). parse("5 m % 2 m") however should work, simply because it makes sense: if I have a log of 5 m and chop it into 2 m pieces, I have 2 pieces (that's the floordiv) and 1 m left over (that's the modulo).

In short: for floordiv and modulo both operands need to have the same dimensions, like for addition or subtraction, otherwise it makes no sense.

When something makes no sense there is the option to define some sense. So one could regard parse("5 m % 2") as equivalent of parse("(5 % 2) m"), but I'm not dogmatic about it.

[tecki]

Well, if you regard it like that, what should the value of parse("5000 mm % 2") be? Given that 5 m and 5000 mm are the same, the result should better be the same.

[hgrecco]

@deeplook I think it is better not to guess the user intentions. We should follow Python's operator precedence link replacing a string by the corresponding quantity object.

[deeplook]

Well, if you regard it like that, what should the value of parse("5000 mm % 2") be? Given that 5 m and 5000 mm are the same, the result should better be the same.

You would stick with the one unit you have, and get a 0 result. Again, I'm not dogmatic on it. Either way it might cause surprises, so let's pick the one closest to the Python conventions.

[hgrecco]

I think this is fixed. Feel free to reopen.

[westurner]

This one is wrong, too, FWIU: hgrecco/pint-pandas#95 (comment) :

I get an error when using the floordiv method on a PintArray with a scalar, see example below:

from pint_pandas import PintArray
import pandas as pd

p = PintArray([1., 2.], dtype="m")
s = pd.Series(p)

s / 2  # works
s // 2  # pint.errors.DimensionalityError: Cannot convert from 'meter' to 'dimensionless'

floordiv with another PintArray is workin

[westurner]

What could a better Exception message read?

[Evidlo]

I don't agree with some of the conclusions in this thread because they are predicated on the idea that all operations should be unit-aware. Specifically this statement:

This issue is blatantly wrong. Floordiv should absolutetly give a DimensionalityError, because it is a dimensionality error. Your example shows this very well. You want to floor-divide 10" by 3. What in world should the result be? Probably you say 3". But this is exactly the dimensionality error that pint tries heavily to prevent you from. Why? Because I guess we agree that 10" = 254 mm. If you floor-divide that by 3 in your logic, you get 84 mm, which is certainly not the 3" you got from before.

floor(3.333 inch) is an unambiguous operation which translates to e.g. "round this 3.333 length of wood down to the nearest foot". floor as a mathematical concept is unit-agnostic.

If you accept the above argument, then 10 inch // 3 it not a unit-aware operation either and should give us 3 inch.

However, I do agree that 10 inch % 3 should throw a DimensionalityError, because a % b translates to "split this length 'a' piece of wood into pieces of length 'b' and give me what is left over".

• floor(10 * u.inch / 3) → 3 <Unit('inch')>
• 10 * u.inch // 3 → 3 <Unit('inch')>
• 10 * u.inch % 3 → DimensionalityError
• divmod(10 * u.inch, 3) → DimensionalityError

[tecki]

Well, if floor is unit-agnostic, as you say, can you please tell me what is floor(3.3 in + 1.1 m)?

[Evidlo]

Well, if floor is unit-agnostic, as you say, can you please tell me what is floor(3.3 in + 1.1 m)?

That doesn't really have anything to do with floor(). The current behavior of Pint when adding compatible units is to use the units from the first summand. So in this case you get
floor(3.3 * u.inch + 1.1 * u.m) → 46 <Unit('inch')>.

[tecki]

Well, if floor is unit-agnostic, as you say, can you please tell me what is floor(3.3 in + 1.1 m)?

That doesn't really have anything to do with floor(). The current behavior of Pint when adding compatible units is to use the units from the first summand. So in this case you get floor(3.3 * u.inch + 1.1 * u.m) → 46 <Unit('inch')>.

Sure. But for your presumably unit-agnostic mathematical function, what is floor(3.3 in + 1.1 m)? Once we know that, let's let pint behave that way, because pint should reflect - as good as reasonably possible - the underlying mathematical concepts.

[deeplook]

Sure. But for your presumably unit-agnostic mathematical function, what is floor(3.3 in + 1.1 m)? Once we know that, let's let pint behave that way, because pint should reflect - as good as reasonably possible - the underlying mathematical concepts.

Maybe this is more about standardization? After all, units are given by physists, say, with different cultural backgrounds before eventually some standardization emerges, if it does. So one could argue that floor in this case returns the value of the standardized base unit, in this case meters as it is the SI standard. Else you can only argue that it will return the sum of two floor operations in their respective units, which appears a little far-fetched.

That said, I kind of dislike that 3.3 * ureg.inch + 1.1 * ureg.meter returns 46.60708661417323 inch. I really think there should be a preference for international standards, but maybe this opens a whole new issue...

[tecki]

Maybe this is more about standardization? After all, units are given by physists, say, with different cultural backgrounds before eventually some standardization emerges, if it does. So one could argue that floor in this case returns the value of the standardized base unit, in this case meters as it is the SI standard. Else you can only argue that it will return the sum of two floor operations in their respective units, which appears a little far-fetched.

Well, in engineering, the standard base unit is certainly the millimeter. Why not that? And for mass, should the standard base unit be kilogram, or gram? And why? What about the volume, cubic meter, or better liter? Note that floor behaves different for all of these.

[Evidlo]

Sure. But for your presumably unit-agnostic mathematical function, what is floor(3.3 in + 1.1 m)?

The answer is in my comment. floor(3.3 in + 1.1 m) = 46 in

I really think there should be a preference for international standards, but maybe this opens a whole new issue...

I agree that this is a separate issue.

Note that floor behaves different for all of these.

The proposed behavior of floor is the same for all of these because it just preserves the unit of its argument.