Skip to content

Conversation

@jl-wynen
Copy link
Member

This finally fixes the last remaining issues in the package source. At least as far as they can be found by checking the source alone. We may find more by checking all tests and downstream packages.

I also started fixing issues in tests. But this is not done yet.

We need to use a union in the tuple to allow passing unions as argument. The overloads only allow for inputs of a single type (int, slice, etc.) but not int|slice.
@nvaytet nvaytet requested a review from Copilot April 11, 2025 20:25
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 98 out of 98 changed files in this pull request and generated no comments.

@nvaytet
Copy link
Member

nvaytet commented Apr 23, 2025

I admire the work put into this.
As mentioned in person, I am wondering if we should have a day-long workshop where we get together and get mypy to pass on all the core packages, so that future fixes would only be incremental (could even enable mypy in CI?).

As for the changes here, I did not look in detail, but Copilot seems happy enough ;-)
I am wondering if some of the changes could have affected performance in any way?
I can't remember; do we have any benchmarks for python as well, or are they only C++ benchmarks?

@jl-wynen
Copy link
Member Author

I ran some simple benchmarks of curve_fit and found no difference. I don't think this PR modified any other code that could have a runtime impact.

@jl-wynen jl-wynen merged commit 783e26c into main Apr 23, 2025
7 of 8 checks passed
@jl-wynen jl-wynen deleted the appease-mypy branch April 23, 2025 12:27
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.

3 participants