Problem X: In python there is no way to enforce that the return value of a method is not discarded, like C++'s [[nodiscard]]
Problem Y: a popular paradigm in Python is to use @classmethod to define an alternative constructor. However, sometimes it’s hard to infer from the method name if such a method is a classmethod, or if it modifies an object in place with the new parameters. Often, users call the classmethod thinking it modifies the calling object in place. For example, deep learning libraries get user issues where they try to load weights for a model, not knowing they called a classmethod, leaving them confused about why their model isn’t in a desired state after calling the classmethod. There’s no mechanism to easily emit a warning or raise an exception to improve usability.
I proposed a solution: for certain classmethod, we raise an exception if the classmethod is called on an instance as a proxy to catch these usage errors.
A pure python implementation:
from typing import TYPE_CHECKING
from types import MethodType
class _restricted_classmethod:
"""
Custom `classmethod` that raises a TypeError when the decorated method
is called on an instance instead of the class type.
"""
def __init__(self, method):
self.method = method
def __get__(self, instance, cls):
if instance is not None:
raise TypeError(
f"The classmethod `{cls.__name__}.{self.method.__name__}` cannot be called on an instance. "
f"Please call it on the class type and make sure the return value is used."
)
return MethodType(self.method, cls)
# currently all static type checkers (pyright, mypy, ...) hard codes `@classmethod` and `@staticmethod`,
# so even with the above decorator, the first argument to the method would be inferred as `Self@T`
# and not `type[T]`, so this is only to make the static type checker happy.
restricted_classmethod = classmethod if TYPE_CHECKING else _restricted_classmethod
class Demo:
@restricted_classmethod
def rcmethod(cls):
pass
Demo.rcmethod() # OK
Demo().rcmethod() # Runtime TypeError
Obviously I would love for there to be an equivalent to [[nodiscard]] in Python. I’d love to hear suggestions on how this could potentially be implemented in static type checking too.
To me, it sounds like the real issue here is the (relatively) novice userbase of pytorch/pytorch lightning. I bet that many users first introduction to Python is through these ML libraries and frameworks. They likely copy and paste a lot and recognize simple patterns that they try to reuse. Classmethods is one of those cases where some simple patterns break. Instead of adding safety rails, this should be considered a learning opportunity for the novice user.
For advanced users that know how to read documentation and code, this is a non issue IMO. Furthermore, it seems Pytorch Lightning uses especially confusing names, which could even trip up more experienced users. Using the same prefix load_from_* for both class and instance methods, instead of the more standard from_* for classmethods is unforturnate design, but not something that requires support from the language IMO.
Thanks for the code snippet. It’s more of a fix with dynamic code than stopping the issue early with a type checker, but can’t the method be removed from an instance, or at least set to None, in its __init__ call ? A restricted class method that creates an instance, could even remove itself from that instance before returning it.
Anyway, why don’t you call it “constructor” instead of “restricted class method”, or is this desirable for other uses than stopping mistaken calls to constructors? If a callable attribute on a class can’t be called on an instance, then is it really still a method at all?
IMO removing the method from the instance is more confusing than raising an exception, since its hides more from the user and is unexpected, and we can’t raise a more user friendly exception/warning.
I don’t think calling it “constructor” is better, since the method doesn’t necessarily have to return an instance of cls.
You’re right - if it can’t be called on an instance, it’s weird to call it a method. Again, this is the Y problem. The X problem IMO is that Python doesn’t have [[nodiscard]]
Good suggestion - I didn’t know this! Removing the method on the instance also removes the opportunity to raise a user friendly exception/warning though.
Have you considered proposing the addition of a typing.no_discard decorator? Then type checkers (like MyPy) or linters (like Ruff) could produce appropriate warnings?
It would remove the method from an instance’s auto-complete in an IDE, though, so the code doesn’t get written in the first place (most of the time). That seems even more desirable than a helpful exception message.
If something like this were to be implemented, I’d prefer an specialized NoDiscard annotation instead of using the generic Annotation. But, I’d much rather prefer not to have this at all, because I don’t think it’s the responsibility of the callee to tell the caller what it can and cannot do with the value produced.
Also, what would the semantics be? Would the following snippet be ok?
def add(a: int, b: int) -> NoDiscard[int]:
return a + b
c = add(1, 3) # the value is assigned but unused later, should the type checker warn?
print("I did a thing!")
I’d much rather let this be a learning opportunity for the new Python user than add a change to the language that doesn’t let me call functions the way I want to.