Skip to content

Conversation

cmp0xff
Copy link
Contributor

@cmp0xff cmp0xff commented Oct 4, 2025

  • Addresses Strict report as of 3/13/2025 #1171.
    • reportPrivateUsage 42. Can we disable this forever? Importing _PrivateObjects is widely used in Pandas. We cannot remove them.
    • reportUnknownLambdaType: 50. Can we disable this forever? They are only in tests at places where one can insert a callable.
    • reportUnknownArgumentType: 118.
    • reportUnknownVariableType: 405.
    • reportMissingTypeArgument: 430.
    • reportMissingParameterType: 799.
    • reportUnknownParameterType: 1235.
    • reportUnknownMemberType: 1411.

If one wants to investigate the errors individually, here's the JSON snippet that can be inserted to pyrightconfig-strict.json. Simply enable the specific categories of error of interest.

{
  "reportUnknownArgumentType": false,
  "reportUnknownVariableType": false,
  "reportMissingTypeArgument": false,
  "reportMissingParameterType": false,
  "reportUnknownParameterType": false,
  "reportUnknownMemberType": false
}

"reportMissingModuleSource": true,
"useLibraryCodeForTypes": false
"useLibraryCodeForTypes": false,
"reportPrivateUsage": false,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need to keep reportPrivateUsage in there. I think pyright is wrong for reporting them in stub files. Created an issue: microsoft/pyright#10992

From the test files, we will be able to fix this once pandas 3.0 is released, where there will be a pandas.api.typing.aliases which will be the supported way for people to import instead of using pandas._typing. One reason I introduced the aliases was to get around this particular warning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added back in 7d3c152

@cmp0xff cmp0xff changed the title chore(pyright): #1171 remove two strict rules chore(pyright): #1171 return None when possible; disable reportUnknownLambdaType Oct 4, 2025
@cmp0xff
Copy link
Contributor Author

cmp0xff commented Oct 4, 2025

/pyright_strict

Copy link
Contributor Author

@cmp0xff cmp0xff left a comment

Choose a reason for hiding this comment

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

  • Also added -> None to tests and other possible places
  • pyright_strict errors reduced from 3732 to 3682.

"reportMissingModuleSource": true,
"useLibraryCodeForTypes": false
"useLibraryCodeForTypes": false,
"reportPrivateUsage": false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added back in 7d3c152

Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

thanks @cmp0xff . With respect to the python functions in .py files returning None, is there some test we can include for new PRs (in pre-commit, or in the type checkers) that would pick that up?

@Dr-Irv Dr-Irv merged commit 10f5f92 into pandas-dev:main Oct 4, 2025
13 of 14 checks passed
@cmp0xff cmp0xff deleted the bugfix/cmp0xff/1171-update branch October 4, 2025 18:22
@cmp0xff
Copy link
Contributor Author

cmp0xff commented Oct 4, 2025

is there some test we can include for new PRs (in pre-commit, or in the type checkers) that would pick that up?

Hi @Dr-Irv, I used ANN rules in ruff. I did unsafe-fixes and then checked manually. Maybe we can also introduce a ruff_strict test in the main branch, including more rules?

@Dr-Irv
Copy link
Collaborator

Dr-Irv commented Oct 4, 2025

Hi @Dr-Irv, I used ANN rules in ruff. I did unsafe-fixes and then checked manually. Maybe we can also introduce a ruff_strict test in the main branch, including more rules?

I'd rather be incremental in adding rules in pyproject.toml for ruff . So if adding ANN works, we can do that, and do it for other rules as well.

@cmp0xff
Copy link
Contributor Author

cmp0xff commented Oct 4, 2025

We are very far from making all ANN rules work.

Alternatively I can propose that we select "ALL" ruff rules and exclude not working rules, which we gradually fix and remove (or decide not to support)

@Dr-Irv
Copy link
Collaborator

Dr-Irv commented Oct 5, 2025

Alternatively I can propose that we select "ALL" ruff rules and exclude not working rules, which we gradually fix and remove (or decide not to support)

I'm fine with that. We already exclude some rules already.

My goal in bringing this up is that in this PR, you changed test functions to return None, so we'd want to make sure that in future PRs, this is automatically caught/fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants