Fixed warning in fourigui.py code #56
Conversation
Your branch and 'origin/warning' have diverged, and have 1 and 1 different commits each, respectively. (use "git pull" if you want to integrate the remote branch with yours) All conflicts fixed but you are still merging. (use "git commit" to conclude merge) Merge branch 'warning' of https://github.com/Filippo1598/diffpy.fourigui into warning
|
|
||
| **Deprecated:** | ||
|
|
||
| * Deprecation warning fixed for axes in np.fft in fourigui.py |
There was a problem hiding this comment.
Typically depreciated refers to classes and functions no longer supported.
Here, we are fixing these annoying warnings. So you can put it under "Fixed" - please refer to the "Lessons from ..." GitLab guide for writing a news file.
There was a problem hiding this comment.
No please do now. These are user facing news.
|
@Fil158 The original issue mentioned here was created to ensure we re-visit to fix these warnings. But, what exact warnings are you fixing and how? Even if it's a simple fix, we will probably forget things later and also it will be useful for future maintainers. |
Thanks for this. Reviewers all can see what were changed. This information has no further value... |
| @@ -394,8 +394,9 @@ def fft(self): | |||
| def perform_fft(fftholder): | |||
| fftholder = np.nan_to_num(fftholder) | |||
| size = list(fftholder.shape) | |||
| axes = list(range(len(size))) | |||
There was a problem hiding this comment.
this is not very clear. In the comments, let's have a brief discussion of what is deprecated in numpy2 and how we are fixing it. Not too much, but presumably @Fil158 you googled something and found something out so it is helpful to share what you found out and a few words about how it affects us here and so your suggested fix. It builds more confidence that we understand what we are doing.
Also, please make your PR title more informative. Which warning does this PR fix? Not too long, but just enough.
I am not sure but there is almost certainly a more readable fix than list(range(len(size)))
There was a problem hiding this comment.
@sbillinge Here is the worning, please see the Numpy official documentation
To solve it I specify axes which is a list of integers with the same size of s
After the changes pytests run correctly and fourigui app works with test data.
There was a problem hiding this comment.
Thanks @Fil158 this is very helpful. Next we need a quick discussion about what the shape of the arrays are and what th axes are and what our desired fft is.
Then finally, what code we think is best to accomplish that.
|
@sbillinge @Fil158 
|
|
I don't know if my inline comments are showing to everyone... so I'm pasting them here. Sorry if you see duplicates. @sbillinge Here's some info from the 
|
|
@cadenmyers13 as we discussed, could you utilize the knowledge here to resolve the warnings in a new PR? |
|
@Fil158 you may close this issue if @cadenmyers13 is willing to do so. |
Closes #22 Warning fixed in fourigui.py adding line 397
axes = list(range(len(size)))and modifying line 399fftholder = np.fft.fftn(fftholder, s=size, axes=axes, norm="ortho")