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

Convert fixes #64

Merged
merged 4 commits into from Feb 15, 2021
Merged

Convert fixes #64

merged 4 commits into from Feb 15, 2021

Conversation

mikapfl
Copy link
Contributor

@mikapfl mikapfl commented Feb 15, 2021

I found a couple of assorted problems while working on my PR, which I propose to fix here.

For the example updates / fixes, I re-ran all examples in the docstrings I found and fixed problems where they wouldn't run and updated the output.

The other fix is just a small change which increases test coverage somewhat because the wrong data type was being tested.

Copy link
Collaborator

@keewis keewis left a comment

Choose a reason for hiding this comment

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

oh, yes, most of these examples were written before we introduced _repr_inline_.

I think it would be good to have a CI that checks the doctests, so we don't regress again. I will merge this first and then add that CI.

pint_xarray/tests/test_conversion.py Outdated Show resolved Hide resolved
@keewis
Copy link
Collaborator

keewis commented Feb 15, 2021

thanks. If you want, you can also add an entry to whats-new.rst, the doctests at least have been broken since before the release of 0.1 so this is user visible.

@mikapfl
Copy link
Contributor Author

mikapfl commented Feb 15, 2021

I added a small changelog entry. 🙂

@keewis
Copy link
Collaborator

keewis commented Feb 15, 2021

thanks, @mikapfl

@keewis keewis merged commit 1102d56 into xarray-contrib:master Feb 15, 2021
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.

None yet

2 participants