Skip to content

TYP: Enable pyright's reportInconsistentConstructor #54398

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 4, 2023
Merged

TYP: Enable pyright's reportInconsistentConstructor #54398

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
44c1a7f
Enable pyright's reportInconsistentConstructor
twoertwein Aug 4, 2023
c028de9
bump pyright without adding new errors
twoertwein Aug 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ repos:
types: [python]
stages: [manual]
additional_dependencies: &pyright_dependencies
- pyright@1.1.296
- pyright@1.1.305
- id: pyright
# note: assumes python env is setup and activated
name: pyright reportGeneralTypeIssues
Expand Down
3 changes: 3 additions & 0 deletions pandas/_typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,3 +463,6 @@ def closed(self) -> bool:

# to_stata
ToStataByteorder = Literal[">", "<", "little", "big"]

# ExcelWriter
ExcelWriterIfSheetExists = Literal["error", "new", "replace", "overlay"]
3 changes: 0 additions & 3 deletions pandas/core/computation/ops.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,6 @@ def ndim(self) -> int:


class Constant(Term):
def __init__(self, value, env, side=None, encoding=None) -> None:
super().__init__(value, env, side=side, encoding=encoding)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not be needed, just calls the parent constructor (and uses value instead of name)


def _resolve_name(self):
return self._name

Expand Down
4 changes: 2 additions & 2 deletions pandas/core/computation/pytables.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ def value(self):


class Constant(Term):
def __init__(self, value, env: PyTablesScope, side=None, encoding=None) -> None:
def __init__(self, name, env: PyTablesScope, side=None, encoding=None) -> None:
assert isinstance(env, PyTablesScope), type(env)
super().__init__(value, env, side=side, encoding=encoding)
super().__init__(name, env, side=side, encoding=encoding)

def _resolve_name(self):
return self._name
Expand Down
2 changes: 1 addition & 1 deletion pandas/core/indexes/accessors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ class PeriodProperties(Properties):
class CombinedDatetimelikeProperties(
DatetimeProperties, TimedeltaProperties, PeriodProperties
):
def __new__(cls, data: Series):
def __new__(cls, data: Series): # pyright: ignore[reportInconsistentConstructor]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parent __init__ expect orig as a second argument. Just ignoring the error as the comment indicates that this class is never "really" instantiated.

# CombinedDatetimelikeProperties isn't really instantiated. Instead
# we need to choose which parent (datetime or timedelta) is
# appropriate. Since we're checking the dtypes anyway, we'll just
Expand Down
5 changes: 3 additions & 2 deletions pandas/io/excel/_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
from pandas._typing import (
DtypeArg,
DtypeBackend,
ExcelWriterIfSheetExists,
FilePath,
IntStrT,
ReadBuffer,
Expand Down Expand Up @@ -1129,7 +1130,7 @@ def __new__(
datetime_format: str | None = None,
mode: str = "w",
storage_options: StorageOptions | None = None,
if_sheet_exists: Literal["error", "new", "replace", "overlay"] | None = None,
if_sheet_exists: ExcelWriterIfSheetExists | None = None,
engine_kwargs: dict | None = None,
) -> ExcelWriter:
# only switch class if generic(ExcelWriter)
Expand Down Expand Up @@ -1218,7 +1219,7 @@ def __init__(
datetime_format: str | None = None,
mode: str = "w",
storage_options: StorageOptions | None = None,
if_sheet_exists: str | None = None,
if_sheet_exists: ExcelWriterIfSheetExists | None = None,
engine_kwargs: dict[str, Any] | None = None,
) -> None:
# validate that this engine can handle the extension
Expand Down
3 changes: 2 additions & 1 deletion pandas/io/excel/_odswriter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

if TYPE_CHECKING:
from pandas._typing import (
ExcelWriterIfSheetExists,
FilePath,
StorageOptions,
WriteExcelBuffer,
Expand All @@ -39,7 +40,7 @@ def __init__(
datetime_format=None,
mode: str = "w",
storage_options: StorageOptions | None = None,
if_sheet_exists: str | None = None,
if_sheet_exists: ExcelWriterIfSheetExists | None = None,
engine_kwargs: dict[str, Any] | None = None,
**kwargs,
) -> None:
Expand Down
3 changes: 2 additions & 1 deletion pandas/io/excel/_openpyxl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from openpyxl.workbook import Workbook

from pandas._typing import (
ExcelWriterIfSheetExists,
FilePath,
ReadBuffer,
Scalar,
Expand All @@ -48,7 +49,7 @@ def __init__(
datetime_format: str | None = None,
mode: str = "w",
storage_options: StorageOptions | None = None,
if_sheet_exists: str | None = None,
if_sheet_exists: ExcelWriterIfSheetExists | None = None,
engine_kwargs: dict[str, Any] | None = None,
**kwargs,
) -> None:
Expand Down
3 changes: 2 additions & 1 deletion pandas/io/excel/_xlsxwriter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

if TYPE_CHECKING:
from pandas._typing import (
ExcelWriterIfSheetExists,
FilePath,
StorageOptions,
WriteExcelBuffer,
Expand Down Expand Up @@ -189,7 +190,7 @@ def __init__(
datetime_format: str | None = None,
mode: str = "w",
storage_options: StorageOptions | None = None,
if_sheet_exists: str | None = None,
if_sheet_exists: ExcelWriterIfSheetExists | None = None,
engine_kwargs: dict[str, Any] | None = None,
**kwargs,
) -> None:
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,7 @@ include = ["pandas", "typings"]
exclude = ["pandas/tests", "pandas/io/clipboard", "pandas/util/version"]
# enable subset of "strict"
reportDuplicateImport = true
reportInconsistentConstructor = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a follow up, could we enable strict mode and then turn off the checks that don't pass (with todos) or are not relevant like we did with mypy?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could enable strict and disable many of its rules (and I think we had that in the past). I think the pyright maintainer recommended that we prioritize enabling reportGeneralTypeIssues (we have a large blacklist in pyright_reportGeneralTypeIssues.json) before enabling strict as it also changes some of its behavior.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay sounds good

reportInvalidStubStatement = true
reportOverlappingOverload = true
reportPropertyTypeMismatch = true
Expand Down
6 changes: 2 additions & 4 deletions pyright_reportGeneralTypeIssues.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
# this becomes obsolete when reportGeneralTypeIssues can be enabled in pyproject.toml
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pyright uses a stricter JSON parser - comments are not part of the JSON syntax.

(There are much newer versions of pyright but there are too many things we would need to change at the moment.)

{
"typeCheckingMode": "off",
"reportGeneralTypeIssues": true,
Expand All @@ -9,12 +8,11 @@
],
"exclude":
[
# exclude tests
"pandas/tests",
# exclude vendored files

"pandas/io/clipboard",
"pandas/util/version",
# and all files that currently don't pass

"pandas/_testing/__init__.py",
"pandas/_testing/_hypothesis.py",
"pandas/_testing/_io.py",
Expand Down