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

Blog post #142

Closed
wants to merge 24 commits into from
Closed

Blog post #142

wants to merge 24 commits into from

Conversation

TomNicholas
Copy link
Member

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.

I have a few comments, but this really looks great so far!

blogpost.md Outdated Show resolved Hide resolved
blogpost.md Outdated Show resolved Hide resolved
blogpost.md Outdated Show resolved Hide resolved
blogpost.md Show resolved Hide resolved
blogpost.md Outdated Show resolved Hide resolved
blogpost.md Outdated Show resolved Hide resolved
blogpost.md Outdated Show resolved Hide resolved
blogpost.md Outdated Show resolved Hide resolved
@keewis
Copy link
Collaborator

keewis commented Nov 1, 2021

I think the doctest failure is a upstream issue: it may have been introduced when I changed the formatting code of pint to allow custom formats...

@keewis
Copy link
Collaborator

keewis commented Nov 1, 2021

I think the doctest failure is a upstream issue: it may have been introduced when I changed the formatting code of pint to allow custom formats...

that's wrong, it is the same issue as above: assigning to the application registry modifies the wrapper and not the unit registry. In this case, instead of force_ndarray_like we tried to set default_format.

@keewis
Copy link
Collaborator

keewis commented Mar 2, 2022

as decided in the meeting this evening, we'll skip the expects decorator for now and update the blogpost to be published soon.

We still need to wait on the pint release (pint=0.19), which should hopefully be released very soon. In addition to a few bug fixes, this will include a better docs theme, which makes the docs much easier to read (actually that's the only blocking PR right now).

blogpost.md Outdated
Comment on lines 109 to 111
@expects("newton * seconds")
def jpl_trajectory_code(impulse):
print(f"Received impulse in units of [{impulse.pint.units}]")
Copy link
Member Author

Choose a reason for hiding this comment

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

Change this to a simple if raise statement for now, and add a note that we will improve this API in future

Copy link
Member Author

Choose a reason for hiding this comment

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

Done now, though I should probably print the actual error message that would be raised

Comment on lines +161 to +165
## Dequantifying

To convert our pint arrays back into numpy arrays, we can use `.dequantify`.
This will strip the units from the arrays and replace them into the `.attrs['units']` of each variable.
This is useful when we want to save our data back to a file, as it means that the current units will be preserved in the attributes of a netcdf file (or zarr store etc.), as long as we just do `ds.pint.dequantify().to_netcdf()`.
Copy link
Member Author

Choose a reason for hiding this comment

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

Put a note in about option of dequantifying to different formats

Copy link
Member Author

Choose a reason for hiding this comment

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

@keewis should we bother with this?

@TomNicholas TomNicholas added this to the v0.3 (inc. blogpost) milestone May 18, 2022
@TomNicholas TomNicholas marked this pull request as ready for review May 18, 2022 18:25
@TomNicholas
Copy link
Member Author

@keewis we should ideally add this content as a new page of examples in the docs

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.

Blog post to show off pint-xarray integration?
2 participants