Skip to content
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

Added windows-latest to the strategy section. #460

Merged
merged 2 commits into from
Nov 20, 2023
Merged

Added windows-latest to the strategy section. #460

merged 2 commits into from
Nov 20, 2023

Conversation

AnshRoshan
Copy link
Contributor

PR Summary

Closes #454
I have added the window-latest in the strategy section in .github/workflows/ci-release.yml and .github/workflows/ci.yml

@kafitzgerald
Copy link
Contributor

Shoot, it looks like the EOF tests are all failing for Windows.

The results seem to have the wrong sign. I'm not sure why this would be the case offhand though. @anissa111 might have a better idea.

@anissa111
Copy link
Member

Interesting! I think fixing this is probably outside the scope of this PR, but I'd like to know what's going on before merging so that we don't have our regular CI failing

@AnshRoshan
Copy link
Contributor Author

I tried to see the error, it is generating negative from the

eofs = solver.eofs(neofs=neofs, eofscaling=eofscaling)

eofs = solver.eofs(neofs=neofs, eofscaling=eofscaling)
,then it went to xarray.py I ,in which I was not able to understand

@kafitzgerald
Copy link
Contributor

Yeah, as Anissa mentioned this is not a direct result of your changes, but a separate issue (uncovered by your changes) that we'll need to sort out before we merge this.

I'll go ahead and log another issue so we can track that separately. Once we have that sorted out, we should hopefully be able to get this merged quickly.

Thanks again for your PR!

@anissa111 anissa111 self-requested a review November 20, 2023 23:18
@anissa111 anissa111 merged commit 9c38bd3 into NCAR:main Nov 20, 2023
12 checks passed
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.

Add windows to testing matrix
3 participants