gis.measure: Distance/Distance should error.

[Robert Coup]

The original design principle of the measure module were to:

1. minimise unit mismatching errors
2. avoid bad maths on units.
3. provide helpful converters so everyone doesn't have constants all over their codebases

With respect to (2), for example: (Area * Distance) is a meaningless thing - m³

While Distance / Distance (& Area / Area) do produce ratios, they are quite likely to be an error.

Originally Distance / Distance would raise a TypeError, same as Area / Area still does currently. This was changed during a refactoring a while back (#17754).

Suggestion to resolve:

1. Add a MeasureBase.ratio(other) method, which makes it really explicit what is happening
2. Make Distance / Distance raises a DeprecationWarning, and eventually go back to raising a TypeError
3. Alias ratio() to another operator (eg. %) I think it's not used enough to actually be clearly readable to anyone.

[Mariusz Felisiak]

If I understand correctly, you propose to remove a feature added in #17754 because you think it's unnecessary or maybe unclear. I don't see any issues in supporting Distance / Distance or Distance * Distance as far as I'm concerned they work fine:

>>> d1 = Distance(m=100)
>>> d2 = Distance(km=1)
>>> d2/d1
10.0
>>> d1/d2
0.1
>>> d1 * d2
Area(sq_m=100000.0)

I also don't think that would be better (or more readable) to add a new API (ratio()) for this.

[Robert Coup]

If I understand correctly, you propose to remove a feature added in #17754 because you think it's unnecessary or maybe unclear.

Not saying that at all.

When I wrote measure originally, one goal was trying to make error-prone handling of dimensioned values safer. That's the reasoning you can't multiply Distance * Area, for example. The same reasoning you can't do MyModel.objects.delete(), you need to explicitly use .all().delete(): for safety.

It was changed in #17754 without any discussion that I can find, as part of the bigger refactoring effort. I suggest the ticket is re-opened for discussion as a bug.

I don't see any issues in supporting Distance / Distance or Distance * Distance as far as I'm concerned they work fine:

Yes, Distance/Distance returns a number, but it should raise a TypeError. My suggestion was that if the user wants a ratio, then they should have to explicitly ask for a ratio.

(Distance * Distance should definitely return an Area, that's the simplest definition of an Area!)

Alternatively, if unit safety is no longer a goal of the module, then the TypeError raised by Area/Area should be dropped, and arguably many of the other TypeErrors, and the documentation should have a big "buyer beware, we don't help prevent bad dimensioned maths anymore" statement added.

[Mariusz Felisiak]

Yes, Distance/Distance returns a number, but it should raise a TypeError.

That's something that I'm missing, why it should raise an error? Do you have examples of incorrect calculations?

... buyer beware...

Off-topic, as far as I'm concerned we don't have buyers :) .

[Mariusz Felisiak]

Claude, Can you take a look?

[Robert Coup]

Yes, Distance/Distance returns a number, but it should raise a TypeError.

That's something that I'm missing, why it should raise an error? Do you have examples of incorrect calculations?

Sure, it prevents a class of programming errors for incompatible unit conversions. The ​original paper gis.measure was based on discusses:

... Such a system should also support units, which give meaning to dimensioned data. Units support also prevents inappropriate mixing of incompatible dimensioned values, for example distance and time.
Existing robot programming systems typically usefixed or floating point numeric types to represent dimensioned data, particularly geometric data, such as those in Table 1. Units are not specified, but an implied standard unit is assumed and the programmer is responsible for ensuring all necessary conversions.
It is not safe to rely on the programmer to ensure unit conversions, especially where different unit standards are used. In larger projects, the difficulties are compounded since many programmers are involved. It is difficult to change the implied standard units; the recent Player and Stage unit change from millimetres to metres generated much discussion and considerable reengineering of clients. Another, well-known incident was the failure of NASA’s Mars Climate Orbiter mission; the root cause was data with incorrect units (9).
...
The dimensional analysis technique can help make programs more robust against errors in units by verifying the consistency of the units given. It allows a new class of errors [incompatible conversions] to be caught (10). It can be applied by adding a new attribute to data types of the dimensions and units used, enabling verification of data compatability and automatic conversions.
...
A key part of dimensional analysis is unit algebra: the interaction of the units when dimensioned values are combined. It prevents incompatible conversions (e.g. seconds to millimetres) while correctly transforming appropriate combinations (e.g. metres divided by seconds gives a value in metres per second). Unit algebra must be active for all dimensioned values.

Now, Unit / Unit does produce a ratio, which can be useful in some scenarios (and Distance/Distance does correctly return a float rather than another Distance), though is often an accident. But whichever way it goes we should be consistent across Distance & Area.

... buyer beware...
Off-topic, as far as I'm concerned we don't have buyers :) .

The module did protect users, now it doesn't. IMO the same as if Django decides to make MyModel.objects.delete() work — if the ORM allowed that everything would "still work", but it would be a "bug" as a design principle violation.

[Carlton Gibson]

Hi Robert, thanks for your comments.

The change here was 7 years ago... "did protect users, now it doesn't" seems to be stretching it. :)

The changes you describe were a deliberate design choice, not just an unrelated result of the refactoring. (See e.g. https://code.djangoproject.com/ticket/17754#comment:7)
As such we'd need a much fuller discussion than we can have on the issue tracker. ​See the Triage Workflow for more.

For what it's worth here, I think that assuming that users know what they're doing when do unit math is the right way to go.

Personally, I want to be able to use division when I need do. I neither want nor need training-wheels that force me to go via a wrapper function.
(I might add such for learning, but even then, a big part of learning is finding out, experimentally, when operations don't apply...)

I hope that makes sense.
Thanks again.

[Robert Coup]

The change here was 7 years ago... "did protect users, now it doesn't" seems to be stretching it. :)

Ha! Didn't protect me yesterday, which is why I ended up here again ;-)

The changes you describe were a deliberate design choice, not just an unrelated result of the refactoring.

Good spot, I obviously missed (3) reading yesterday.

For what it's worth here, I think that assuming that users know what they're doing when do unit math is the right way to go.

Sure, I've said my piece.

Okay for a separate ticket & patch to remove the TypeError for Area/Area? No reason for it to be inconsistent with Distance. I'll add a brief note to the docs to explain what the module does/doesn't try and help developers with as well.

In [4]: A(sq_m=100) / A(sq_m=10)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-4-d578da15a196> in <module>
----> 1 A(sq_m=100) / A(sq_m=10)

/usr/local/lib/python3.7/site-packages/django/contrib/gis/measure.py in __truediv__(self, other)
    326             )
    327         else:
--> 328             raise TypeError('%(class)s must be divided by a number' % {"class": pretty_name(self)})
    329
    330

TypeError: Area must be divided by a number