[Django] #34676: Add DistanceUnitDoesNotExist exception to contrib.gis.measure

[Django]

#34676: Add DistanceUnitDoesNotExist exception to contrib.gis.measure
------------------------------------------------+--------------------------
Reporter: Andrew Northall | Owner: nobody
Type: Cleanup/optimization | Status: assigned
Component: GIS | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+--------------------------
I am making use of the `Distance` class in `contrib.gis.measure` in my
project and it is annoying to have to constantly catch `Exception` when
checking if a particular unit exists. It would be better to have a
specific exception for this case.

--
Ticket URL: <https://code.djangoproject.com/ticket/34676>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#34676: Add DistanceUnitDoesNotExist exception to contrib.gis.measure

-------------------------------------+-------------------------------------
Reporter: Andrew Northall | Owner: Andrew
Type: | Northall

Cleanup/optimization | Status: assigned
Component: GIS | Version: dev

Severity: Normal | Resolution:

Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Andrew Northall):

* cc: Andrew Northall (added)
* owner: nobody => Andrew Northall

Old description:

> I am making use of the `Distance` class in `contrib.gis.measure` in my
> project and it is annoying to have to constantly catch `Exception` when
> checking if a particular unit exists. It would be better to have a
> specific exception for this case.

New description:

I am making use of the `Distance` class in `contrib.gis.measure` in my
project and it is annoying to have to constantly catch `Exception` when
checking if a particular unit exists. It would be better to have a
specific exception for this case.

https://github.com/django/django/pull/17006

--

--
Ticket URL: <https://code.djangoproject.com/ticket/34676#comment:1>

[Django]

#34676: Add DistanceUnitDoesNotExist exception to contrib.gis.measure
-------------------------------------+-------------------------------------
Reporter: Andrew Northall | Owner: Andrew
Type: | Northall
Cleanup/optimization | Status: assigned
Component: GIS | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak):

I don't think that separate exception class is justified, I'd change to
the `ValueError`.

--
Ticket URL: <https://code.djangoproject.com/ticket/34676#comment:2>

[Django]

#34676: Add DistanceUnitDoesNotExist exception to contrib.gis.measure
-------------------------------------+-------------------------------------
Reporter: Andrew Northall | Owner: Andrew
Type: | Northall
Cleanup/optimization | Status: assigned
Component: GIS | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Andrew Northall):

Replying to [comment:2 Mariusz Felisiak]:

> I don't think that separate exception class is justified, I'd change to
the `ValueError`.

Thanks - if you'd rather not have a class, would it perhaps be better to
normalise all unit not found exceptions to `AttributeError`?

`default_units()` already returns `AttributeError` (before I changed it in
my PR), so would it be better just to make `unit_attname()` also return
`AttributeError` to match (and hence retain backwards compatibility with
people who may depend on `default_units()` returning `AttributeError`?

Or would you leave `default_units()` as `AttributeError` and have
`unit_attname()` return `ValueError`?

--
Ticket URL: <https://code.djangoproject.com/ticket/34676#comment:3>

[Django]

#34676: Add DistanceUnitDoesNotExist exception to contrib.gis.measure
-------------------------------------+-------------------------------------
Reporter: Andrew Northall | Owner: Andrew
Type: | Northall
Cleanup/optimization | Status: assigned
Component: GIS | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak):

Replying to [comment:3 Andrew Northall]:

> Replying to [comment:2 Mariusz Felisiak]:
> > I don't think that separate exception class is justified, I'd change
to the `ValueError`.
>
>
> Thanks - if you'd rather not have a class, would it perhaps be better to
normalise all unit not found exceptions to `AttributeError`?
>

> `default_units()` already returns `AttributeError` if the units are not
found (before I changed it in my PR), so would it be better just to make

`unit_attname()` also return `AttributeError` to match (and hence retain
backwards compatibility with people who may depend on `default_units()`

returning `AttributeError`)?

>
> Or would you leave `default_units()` as `AttributeError` and have
`unit_attname()` return `ValueError`?

We should unify both exceptions. I'd use `AttributeError` as it will be
backward compatible.

--
Ticket URL: <https://code.djangoproject.com/ticket/34676#comment:4>