Skip to content

allow specify dtype for fftfreq/rfftfreq #9126 #9150

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

Closed
wants to merge 1 commit into from

Conversation

shitian-ni
Copy link
Contributor

Now able to specify a dtype for fftfreq/rfftfreq, mentioned in issue #9126

@shitian-ni shitian-ni changed the title changed to specify dtype for fftfreq/rfftfreq #9126 allow specify dtype for fftfreq/rfftfreq #9126 May 21, 2017
Copy link
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

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

This is not a good approach to passing dtypes - why not just let the user pass np.int16 directly, rather than the number 1?

@shitian-ni
Copy link
Contributor Author

@eric-wieser I see, will fix that. Thanks for the review.
By the way, can I add a function in the file for the many "if t==" and "elif t=="?

@eric-wieser
Copy link
Member

eric-wieser commented May 21, 2017

No, because that if statement should not exist in the first place. I don't like this patch at all - right now its just a very convoluted way of spelling np.fftfreq(...).astype(np.int16) as np.fftfreq(..., t=1). IMO, that's not an improvement

@shitian-ni
Copy link
Contributor Author

Oh, I didn't know the astype solution.
It's very true being convoluted.
I will close this PR.

@shitian-ni shitian-ni closed this May 21, 2017
@eric-wieser
Copy link
Member

Looking at the referenced issue, I think the actual request is that the output type match the type of d

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