Skip to content
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

complete scipy.fft.dct, and add relevant type aliases #118

Merged
merged 4 commits into from
Oct 21, 2024

Conversation

Avasam
Copy link
Contributor

@Avasam Avasam commented Oct 20, 2024

This is the only function used by https://github.com/Toufool/AutoSplit, and I'd gladly replace my local https://github.com/Toufool/AutoSplit/tree/9588834e0881e8c09b449954f4c5c24f0e15bd14/typings/scipy for a community-based one.

I've also added and applied type aliases for "DCT type" and "normalization mode"

I'm not 100% certain about the numpy generic types.

@Avasam Avasam changed the title Annotate scipy.fft._realtransforms.dct Annotate scipy.fft._realtransforms.dct and add relevant type aliases Oct 20, 2024
Copy link
Owner

@jorenham jorenham left a comment

Choose a reason for hiding this comment

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

Thank you for this! Every improvement helps, so it's no problem to specifically target a single function like this 😄

I left a few suggestions on how to improve this.
The reason a can't accept this as-is, is because the current dct annotations could lead to incorrectly inferred types and false positives. See my inline dct comment for why this is.

scipy-stubs/fft/_realtransforms.pyi Outdated Show resolved Hide resolved
scipy-stubs/_typing.pyi Show resolved Hide resolved
scipy-stubs/fft/_realtransforms.pyi Outdated Show resolved Hide resolved
scipy-stubs/fft/_realtransforms.pyi Outdated Show resolved Hide resolved
@jorenham jorenham changed the title Annotate scipy.fft._realtransforms.dct and add relevant type aliases complete scipy.fft.dct, and add relevant type aliases Oct 20, 2024
@Avasam Avasam requested a review from jorenham October 21, 2024 01:49
@jorenham jorenham mentioned this pull request Oct 21, 2024
@jorenham jorenham merged commit 457ef12 into jorenham:master Oct 21, 2024
2 checks passed
@jorenham
Copy link
Owner

Thanks, @Avasam!

@Avasam Avasam deleted the patch-1 branch October 21, 2024 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants