xgcm / xrft

Fourier transforms on xarray data structures
https://xrft.readthedocs.io/en/latest/
MIT License
154 stars 45 forks source link

Readded test_xrft.py #174

Closed avik2007 closed 2 years ago

avik2007 commented 2 years ago

I fixed a typo in cross_spectrum that made using window_correction = True unusable with the function. I added a section to test_cross_spectrum that should test that this fix works.

avik2007 commented 2 years ago

Thanks - I was trying to figure out what the problem was here.

Also, I haven't been able to push a few recent change attempts due to the formatting with "black" failing. I also haven't been able to reload the testing environment. Would the testing environment affect the formatting success?

On Sun, Jan 16, 2022 at 2:56 PM Takaya Uchida @.***> wrote:

@.**** requested changes on this pull request.

In xrft/tests/test_xrft.py https://github.com/xgcm/xrft/pull/174#discussion_r785487427:

@@ -542,7 +542,7 @@ def test_cross_spectrum(self, dask): window_correction=True, ) test = (daft * np.conj(daft2)).values / N ** 4

The errors made it past the AttributeError. I think we're almost at the finish line to merge this. The current error is stemming from test losing its metadata. Can you remove .values? I think the test should work then :)

— Reply to this email directly, view it on GitHub https://github.com/xgcm/xrft/pull/174#pullrequestreview-853714255, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHDHETIM26LDT7CY74VDGU3UWMPFNANCNFSM5L47RTNA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

roxyboy commented 2 years ago

Also, I haven't been able to push a few recent change attempts due to the formatting with "black" failing. I also haven't been able to reload the testing environment. Would the testing environment affect the formatting success?

I don't think so... What's the Black error you're getting?

avik2007 commented 2 years ago

Hi Takaya,

Funny enough, I was just able to commit and push through an update. Apparently the error is gone now. Hopefully this passes the tests. Best, Avik

On Sun, Jan 16, 2022 at 3:59 PM Takaya Uchida @.***> wrote:

Also, I haven't been able to push a few recent change attempts due to the formatting with "black" failing. I also haven't been able to reload the testing environment. Would the testing environment affect the formatting success?

I don't think so... What's the Black error you're getting?

— Reply to this email directly, view it on GitHub https://github.com/xgcm/xrft/pull/174#issuecomment-1013951306, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHDHETN3YJ4PL5F5CX2PUWLUWMWT7ANCNFSM5L47RTNA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

avik2007 commented 2 years ago

@roxyboy I'm sorry I'm not able to see what the change you're requesting is.

roxyboy commented 2 years ago

The tests have all passed so I'm going to merge this. Thanks for your work @avik2007 ! :)

avik2007 commented 2 years ago

Thanks for all your help!

On Sun, Jan 16, 2022 at 5:22 PM Takaya Uchida @.***> wrote:

Merged #174 https://github.com/xgcm/xrft/pull/174 into master.

— Reply to this email directly, view it on GitHub https://github.com/xgcm/xrft/pull/174#event-5900878732, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHDHETJSY3DWZNBAD7VO3GTUWNAK5ANCNFSM5L47RTNA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>