https://pantsbuild.org/ logo
#general
Title
# general
c

curved-television-6568

11/24/2021, 2:11 PM
@hundreds-father-404 Is the
OverridesField
meant to always be subclassed?
https://github.com/pantsbuild/pants/blob/897f5c8bb62133fce532771e8bc56cef2cc0626f/src/python/pants/engine/target.py#L2220 I see this, when I didn’t:
Copy code
$ ./pants help
15:12:04.38 [ERROR] type object 'OverridesField' has no attribute 'help'
So, if so, thinking if there could be a check or some such, that catches this earlier…
Or, just mention it in the class doc string could be enough 😛 (it is pretty obvious, that you need to subclass it in order to set a proper help text)
h

hundreds-father-404

11/24/2021, 2:33 PM
Yeah needs to be subclassed
c

curved-television-6568

11/24/2021, 2:42 PM
Yeah, I’m being dumb..
f

fast-nail-55400

11/24/2021, 3:18 PM
can mypy be configured or type-hinted to detect the issue?
c

curved-television-6568

11/24/2021, 3:22 PM
was hoping that
help
could be annotated such, that it would err if not present, for instance.
f
although, the way that the mypy docs describe using
@abstractmethod
, the check doesn’t happen until runtime
c

curved-television-6568

11/24/2021, 3:35 PM
yeah, those all handle methods and properties, not attributes (or classvar’s) from what I can tell..
although, the way that the mypy docs describe using 
@abstractmethod
, the check doesn’t happen until runtime
Yeah, the reason for it being, that a class is deemed abstract if you don’t implement one of the inherited abstract methods, so it’s not an error until you instantiate it (but that’s not restricted to runtime, right?)
f

fast-nail-55400

11/24/2021, 3:39 PM
yeah I saw that. I still think
mypy
should be able to know that the derived type is actually abstract and see that it is being instantiated and should error. Static type information should allow it to make that determination.
An alternative might be us writing a Pants-specific mypy plugin. https://mypy.readthedocs.io/en/latest/extending_mypy.html#extending-mypy-using-plugins
Then we could implement Pants-specific checks.
c

curved-television-6568

11/24/2021, 3:41 PM
Yeah, that could be a way.
I think that a
y: ClassVar[T]
should be enough, as soon as you instantiate a class,
y
should be defined with a value of type
T
, or err out.
f

fast-nail-55400

11/24/2021, 3:45 PM
if mypy didn’t actually consider classes not defining the
ClassVar
as abstract :(
c

curved-television-6568

11/24/2021, 5:23 PM
Yeah, perhaps it would make sense to have a type hint that says a class is not abstract.