Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider Abstract Base Classes for Quantity and Unit #3650

Open
mhvk opened this issue Mar 31, 2015 · 7 comments
Open

Consider Abstract Base Classes for Quantity and Unit #3650

mhvk opened this issue Mar 31, 2015 · 7 comments

Comments

@mhvk
Copy link
Contributor

mhvk commented Mar 31, 2015

Following discussion on how to let Time interact with units without relying overly on ducktyping, @embray suggested (#3648 (comment)) a Quantity abstract base class, with which objects could register to let other code know they can be treated as quantities with a unit. For Unit itself, this would also be useful, in particular for things like functional units which share many but not all of the properties of normal units (see
#1894 (comment)). This issue is to remind us of these thoughts.

p.s. Such a base class might also go quite a long way towards ensuring interoperability with other unit implementations (such as that by amuse).

@embray
Copy link
Member

embray commented Mar 31, 2015

To be clear, I didn't suggest a _meta_class, just an abstract base class.

@embray
Copy link
Member

embray commented Mar 31, 2015

By the way, if we created an ABC for Quantity-like, you can define something like:

@classmethod
def __instancecheck__(cls, inst):
    if hasattr(inst, 'unit') and isinstance(inst.unit, Unit):
        return True
    else:
        return super(QuantityLike, cls).__instancecheck__(inst)

That way isinstance(obj, QuantityLike) will work even for objects of classes that don't explicitly inherit from QuantityLike (as long as they've been registered with QuantityLike.register()). If you want to require a .to() method that's fine too, though you would also want to define that as an abstractmethod.

And yeah, if you wanted to extend the concept to optionally support other unit frameworks that might work too.

@embray embray added the units label Mar 31, 2015
@mhvk mhvk changed the title Consider Meta-classes for Quantity and Unit Consider Abstract Base Classes for Quantity and Unit Apr 1, 2015
@mhvk
Copy link
Contributor Author

mhvk commented Apr 1, 2015

@embray - don't know what it is that I keep mixing up the Meta and Abstract Base Classes; they really have nothing to do with each other! Maybe it is just that I never had to think about Meta classes yet. Anyway, now updated the title and first entry, so that it is clear what this is about.

I think for units there is a bit more of a need: I rather dislike the duck-typing I had to use in #1894.

@embray
Copy link
Member

embray commented Apr 1, 2015

@mhvk Maybe it's because usually to make an ABC you use the ABCMeta metaclass--I get tonguetied around that sometimes too.

@mdboom
Copy link
Contributor

mdboom commented Apr 3, 2015

👍 I think this would make things clearer.

@mhvk
Copy link
Contributor Author

mhvk commented Apr 13, 2018

Issue #6517 reminded me that this is still useful. As @maxnoe wrote:
"""
The base siunits are of class IrreducibleUnit, which does not inherit from Unit, which leads to some highly unexpected results.

>>> isinstance(u.m, u.Unit)
False
>>> isinstance(u.cm, u.Unit)
True
>>> isinstance(u.V, u.Unit)
True
>>> isinstance(u.A, u.Unit)
False

"""
An ABC would not necessarily solve this directly, but at least we could tell people what isiinstance to use.

@nstarman
Copy link
Member

nstarman commented May 2, 2024

@mhvk moving my comment here.

One larger comment, perhaps for @mhvk. Our unit system doesn't have a singular base class. We often write UnitBase, but not everything inherits from it. Perhaps we should add a base class from which all unit classes (including UnitBase) inherit. I would suggest calling it AbstractUnit as it's an ABC and this follows the abstract/concrete naming distinction. This will enable correct typing without impacting existing code. I don't think this should impact this Pull Request as it's very easy to do a find-replace on : UnitBase

I think it would be easy to write an ABC with nothing on it, then add abstractmethods as we encounter them while typing. e.g. as noticed in @eerovaher's PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants