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
Demystify get_all_scoped_flag_names mypy kludges. #10459
Demystify get_all_scoped_flag_names mypy kludges. #10459
Conversation
These are now centralized in a Protocol that takes the place of the prior Callable of two different signatures (!) and a type ignore. The introduced Protocol is a bit klunky, but self contained and can carry the relevant issue pointer in one spot. [ci skip-rust-tests]
# Rust tests will be skipped. Delete if not intended. [ci skip-rust-tests]
# Rust tests will be skipped. Delete if not intended. [ci skip-rust-tests]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much better! Thank you.
# N.B.: We use this callable protool instead of Callable directly to work around the | ||
# dataclass-specific issue described here: https://github.com/python/mypy/issues/6910 | ||
class FlagNameProvider(Protocol): | ||
def __call__(self) -> Iterable: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even better would be to set the return type as Sequence["Options.ScopedFlagNameForFuzzyMatching"]
. We used to have this, but I had to remove it when we added file dependencies because we started erroring on a cycle with options.py
. Now, we tolerate those cycles, so we can add that import back with:
from typing import TYPE_CHECKING
if TYPE_CHECKING:
import pants.option.options import Options
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even better would not be to have an un-needed cycle and that's easy. I'll follow up instead of snowballing this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed :) Thanks!
Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
These are now centralized in a Protocol that takes the place of the
prior Callable of two different signatures (!) and a type ignore. The
introduced Protocol is a bit klunky, but self contained and can carry
the relevant issue pointer in one spot.
[ci skip-rust-tests]