Enable Pyrefly type checking #3474
Conversation
github-actions
bot
added
module: engine
There was a problem hiding this comment.
Thanks for the PR @rchen152!
I was going through the ignored errors trying to understand the issues and left a number of questions (I haven't checked all of them). It would be nice to be able to reduce the number of # pyrefly: ignore lines.
| uv pip install -r requirements-dev.txt | ||
| uv pip install . | ||
| uv pip install mypy | ||
| uv pip install pyrefly==0.44.2 |
There was a problem hiding this comment.
Can we install the latest (and avoid upgrading the version)?
|
|
||
| @trainer.on(Events.EPOCH_STARTED) | ||
| def distrib_set_epoch(engine: Engine) -> None: | ||
| # pyrefly: ignore [missing-attribute] |
There was a problem hiding this comment.
@rchen152 do you know why there is an error here?
train_sampler is checked above to be a DistributedSampler, which has set_epoch method: https://github.com/pytorch/pytorch/blob/d38164a545b4a4e4e0cf73ce67173f70574890b6/torch/utils/data/distributed.py#L139
| removed_in = "0.6.0" | ||
| deprecation_warning = ( | ||
| f"{__file__} has been moved to /ignite/handlers/base_logger.py" | ||
| # pyrefly: ignore [redundant-condition] |
There was a problem hiding this comment.
I agree with pyrefly here, can you please rewrite below line as
- + (f" and will be removed in version {removed_in}" if removed_in else "")
+ + (f" and will be removed in version {removed_in}")and remove the ignore line
| # From pytorch/xla if `torch_xla.distributed.parallel_loader.MpDeviceLoader` is not available | ||
| def __init__(self, loader: Any, device: torch.device, **kwargs: Any) -> None: | ||
| self._loader = loader | ||
| # pyrefly: ignore [read-only] |
There was a problem hiding this comment.
@rchen152 can you please explain this error? I'm not totally sure what is read-only here.
| self._init_from_context() | ||
|
|
||
| def _create_from_backend(self, backend: str, **kwargs: Any) -> None: | ||
| # pyrefly: ignore [bad-override] |
There was a problem hiding this comment.
Same here, rather unclear what is bad-override here and below.
| try: | ||
| import numpy as np | ||
|
|
||
| # pyrefly: ignore [bad-argument-type] |
There was a problem hiding this comment.
Do you know what's wrong here?
| "https://github.com/pytorch/ignite/issues/new/choose" | ||
| ) | ||
|
|
||
| # pyrefly: ignore [unbound-name] |
There was a problem hiding this comment.
Here, I agree with pyrefly, in a follow-up PR we can put start_time before the try/except clauses.
| raise ValueError("Argument every should be integer and greater than zero") | ||
|
|
||
| if once is not None: | ||
| # pyrefly: ignore [unsupported-operation] |
There was a problem hiding this comment.
A bit unclear what's wrong here
2 participants
Ref #3473
Description:
Enables Pyrefly, a fast type checker. I ported the
mypy.inifile to apyrefly.tomlusing thepyrefly initcommand and added# pyrefly: ignorecomments to suppress new type errors.Since this PR already touches a lot of files due to the suppressions, I opted to keep it focused on adding Pyrefly. If it's accepted, I'll follow up with another PR to remove Mypy, update documentation and other references, and clean up easy-to-fix suppressions.