type Command.force#382
Conversation
| self.link_objects = None | ||
| self.debug = None | ||
| self.force = None | ||
| self.force: bool = None # Should always be set in finalize_options |
There was a problem hiding this comment.
Not actually type-safe and easy to break in the future, but this is only place in distutils we assume a type for after finalize_options.
Alternatively this can be kept as bool | None, which still works everywhere expecting a falsy/truthy value. But in some places where an actual boolean is expected (like https://github.com/pypa/setuptools/blob/d198e86f57231e83de87975c5c82bc40c196da79/setuptools/command/build_ext.py#L225-L227 ) we'd have to coerce to a boolean: bool(self.force)
| # function) -- authors of new compiler interface classes are | ||
| # responsible for updating 'compiler_class'! | ||
| compiler_type: ClassVar[str] = None # type: ignore[assignment] | ||
| compiler_type: ClassVar[str] = None |
There was a problem hiding this comment.
Type ignore is accurate, but technically we ignore all assignment atm, will be added back with #368
Avasam
force-pushed
the
type-Command.force
branch
3 times, most recently
from
4000f6d to
c2328c7
Compare
Avasam
force-pushed
the
type-Command.force
branch
from
c2328c7 to
b90852e
Compare
| self.build_lib: str = None # Should always be set in finalize_options | ||
| self.plat_name: str = None # Should always be set in finalize_options | ||
| self.build_temp: str = None # Should always be set in finalize_options |
There was a problem hiding this comment.
@Avasam This change was necessary to get the build to pass. Is this also acceptable?
There was a problem hiding this comment.
Not quite:
- By adding
-> Nonetoinitialize_options, mypy now parses and type-checks this function. So ALL members you initialize and don't explicitly type can only ever beNone. (I've very purposefully avoided dealing with that until more of the project's typing could be cleared up) (surely one of the mypy PRs still left would've detected that issue) - The annoying thing with this initialize/finalize setup in general, is that you get false-positives comparing against
Noneinfinalize_options. Including user-defined overloads offinalize_options.
For now, I've been fixing types toX | Nonewhen it could still beNoneafterfinalize_options. And fixing badNonedefaults when that was appropriate. The third case (as per point 1) I've mostly left alone for now ^^"
If you HAVE to decide, I'd say keeping the type accurate to AFTER running finalize_options is best, and it'll just be a known distutils/setuptools typing caveat that there will be false-positives when comparing against None in initialize_options/finalize_options. (so what you have right now, but type ALL members before adding -> None to initialize_options)
But all is not lost, there's one more trick you can use we developed in typeshed: MaybeNone aka "The Any Trick"
- https://typing.python.org/en/latest/guides/writing_stubs.html#the-any-trick
- https://github.com/python/typeshed/blob/eda55c02d0868770a06b6dad9f74ab608ca7799a/stdlib/_typeshed/__init__.pyi#L55-L59
(it'll obviously reduce type safety, but false-negatives are generally preferred over false-positives, especially for a legacy system like this)
There was a problem hiding this comment.
In case of doubt, let's keep working through other PRs and I can tackle this more in depth
There was a problem hiding this comment.
I'm not sure I follow, but this does allow the tests to pass, so let's go with it for now and we can refine subsequently. Thanks.
There was a problem hiding this comment.
See the new failing test results in #368 for what I meant
distutils/command/build_ext.py:114: error: Incompatible types in assignment (expression has type "None", variable has type "str") [assignment]
distutils/command/build_ext.py:115: error: Incompatible types in assignment (expression has type "None", variable has type "str") [assignment]
distutils/command/build_ext.py:116: error: Incompatible types in assignment (expression has type "None", variable has type "str") [assignment]
distutils/command/build_ext.py:128: error: Incompatible types in assignment (expression has type "None", variable has type "bool") [assignment]
distutils/command/build_ext.py:176: error: Incompatible types in assignment (expression has type "str | None", variable has type "None") [assignment]
distutils/command/build_ext.py:178: error: Incompatible types in assignment (expression has type "list[Extension] | None", variable has type "None") [assignment]
distutils/command/build_ext.py:185: error: Incompatible types in assignment (expression has type "list[str]", variable has type "None") [assignment]
distutils/command/build_ext.py:206: error: Incompatible types in assignment (expression has type "list[Never]", variable has type "None") [assignment]
distutils/command/build_ext.py:208: error: Incompatible types in assignment (expression has type "list[Never]", variable has type "None") [assignment]
distutils/command/build_ext.py:213: error: Incompatible types in assignment (expression has type "list[Never]", variable has type "None") [assignment]
distutils/command/build_ext.py:280: error: Incompatible types in assignment (expression has type "list[Never]", variable has type "None") [assignment]
2 participants
For pypa/setuptools#4861