https://pantsbuild.org/ logo
Join Slack
Powered by
<@UB2J9BQA0> Is the `OverridesField` meant to alwa...
# general
c
@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
Yeah needs to be subclassed
c
Yeah, I’m being dumb..
f
can mypy be configured or type-hinted to detect the issue?
c
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
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
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
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
if mypy didn’t actually consider classes not defining the 
ClassVar
as abstract :(
c
Yeah, perhaps it would make sense to have a type hint that says a class is not abstract.
Open in Slack
PreviousNext
Powered by