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 warning for NaNs #264

Closed
wants to merge 5 commits into from
Closed

Conversation

Bhairvi23
Copy link

Added warning for the first encountered NaN value.
Closes #200

@observingClouds
Copy link
Owner

Hi @Bhairvi23,
Thank you for your first contribution to xbitinfo 🎉 The PR looks good so far and the failure seems to come from the newest changes to xarray. Before this contribution can be merged we need to do a few more things:

  • remove the .DS_Store file that has been accidentally been committed
  • add your changes to the CHANGELOG.md
  • add a simple test to check whether or not the warning will actually be raised.

@observingClouds
Copy link
Owner

@Bhairvi23 I just updated your branch with a fix for the doctests. Now the tests all pass 🎉 If you could address above mentioned points we are ready to merge this.

@Bhairvi23
Copy link
Author

@observingClouds I am unable to find the .DS_Store file. I can see that there has been some changes but I cant seem to locate it.

@observingClouds
Copy link
Owner

Hi @Bhairvi23 ,
The file is likely hidden on your system. You can see it with ls -a. To remove the file from your PC and the git you could do git rm .DS_store and commit the changes afterwards as usual.

@milankl
Copy link
Collaborator

milankl commented Feb 26, 2024

.DS_Store is a hidden file that macos creates to store information of how to visualise a folder (e.g. list vs icons). It can generally be put into .gitignore

@milankl
Copy link
Collaborator

milankl commented Feb 26, 2024

The .DS_Store file is still in the commits though, @Bhairvi23 can you try to delete it? It doesn't show up for me for some reason...

@observingClouds
Copy link
Owner

Sry @Bhairvi23 I just recognise that this issue will already be addressed with #260 which was unfortunately not linked to #200

@Bhairvi23 Bhairvi23 deleted the CheckforNaNs branch February 27, 2024 00:04
@Bhairvi23 Bhairvi23 restored the CheckforNaNs branch May 4, 2024 09:00
@Bhairvi23 Bhairvi23 deleted the CheckforNaNs branch May 4, 2024 09:05
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.

Check for NaNs and raise warning
3 participants