search

xgcm / xrft

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

Explicit error when coordinates are not numercial #190

Closed lanougue closed 1 year ago

lanougue commented 1 year ago

@roxyboy The checks do not seem to be processed. Any idea why ?

roxyboy commented 1 year ago

@roxyboy The checks do not seem to be processed. Any idea why ?

@jbusecke Any ideas on why the Pytests are halted..?

lanougue commented 1 year ago

@roxyboy The checks do not seem to be processed. Any idea why ?

@jbusecke Any ideas on why the Pytests are halted..?

Hi @roxyboy , Maybe something linked to workflow or rule protection of branches. See link below https://github.com/orgs/community/discussions/26698

lanougue commented 1 year ago

@rabernat @roxyboy , hello guys, it would be nice to have this new xrft release ! Can you have a look on how to resolve this PR checks ? it could ease the final formatting of these modifications. Thanks ! Cheers

cisaacstern commented 1 year ago

Thanks for looping me in here @roxyboy! I've been digging around on SO and the best guess I have so far, based on this post, is that perhaps the fact that this PR's first commit was a Verified commit made via the GitHub web browser is somehow causing problems. One of the suggested solutions in that SO post is to close and then re-open the PR, so I will try that now.

cisaacstern commented 1 year ago

🤔 Ok that did not seem to work...

cisaacstern commented 1 year ago

@roxyboy and I are coordinating offline on this... he has just removed the requirement in the repo settings that status checks must pass before a PR is merged. From further reading on Stack Overflow it seems this requirement can sometimes interfere with CI running, as we are observing here. I will now close and re-open this PR to re-trigger CI.

jbusecke commented 1 year ago

Ah ok, sorry then Ill close #193

cisaacstern commented 1 year ago

Looks like that unblocked us! 🎉

Screen Shot 2023-02-06 at 2 05 04 PM
roxyboy commented 1 year ago

Looks like that unblocked us! 🎉

Screen Shot 2023-02-06 at 2 05 04 PM

Yes, it's looking promising!

roxyboy commented 1 year ago

For the future, should I just keep "Require status checks to pass" unchecked..?

cisaacstern commented 1 year ago

Ah ok, sorry then Ill close https://github.com/xgcm/xrft/pull/193

@jbusecke thanks! Takaya looped me in and I didn't realize that you'd opened that PR so just decided to "test in production" 😆 .

cisaacstern commented 1 year ago

For the future, should I just keep "Require status checks to pass" unchecked..?

@roxyboy I think this is the easiest path forward for now, yes.

roxyboy commented 1 year ago

@lanougue It seems that the tests are failing from xarray.ufunc being deprecated..?

lanougue commented 1 year ago

@roxyboy yes, this is why I opened PR #191. I will do both of these modifications in this PR in order to make the tests to pass.

lanougue commented 1 year ago

@roxyboy I think this is it !