Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions news/warning.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
**Added:**

* <news item>

**Changed:**

* <news item>

**Deprecated:**

* Deprecation warning fixed for axes in np.fft in fourigui.py
Copy link
Contributor

@bobleesj bobleesj Nov 6, 2024

Choose a reason for hiding this comment

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

@Fil158

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.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, next time I'll do so

Copy link
Contributor

Choose a reason for hiding this comment

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

No please do now. These are user facing news.


**Removed:**

* <news item>

**Fixed:**

* <news item>

**Security:**

* <news item>
3 changes: 2 additions & 1 deletion src/diffpy/fourigui/fourigui.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Copy link
Contributor

Choose a reason for hiding this comment

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

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)))

Copy link
Author

@Fil158 Fil158 Nov 7, 2024

Choose a reason for hiding this comment

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

@sbillinge Here is the worning, please see the Numpy official documentation

image

To solve it I specify axes which is a list of integers with the same size of s
image

After the changes pytests run correctly and fourigui app works with test data.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

fftholder = np.fft.ifftshift(fftholder)
fftholder = np.fft.fftn(fftholder, s=size, norm="ortho")
fftholder = np.fft.fftn(fftholder, s=size, axes=axes, norm="ortho")
fftholder = np.fft.fftshift(fftholder)
fftholder = fftholder.real
return fftholder
Expand Down