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.

Sign up for GitHub

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

Jump to bottom

Update docstring for compute and persist #8903

Merged
merged 9 commits into from
Apr 2, 2024
Merged

Update docstring for compute and persist #8903

merged 9 commits into from
Apr 2, 2024

Conversation

saschahofmann
Copy link
Contributor

  • Updates the docstring for persist to mention that it is not altering the original object.
  • Adds a return value to the docstring for compute and persist on both Dataset and DataArray

@saschahofmann
Copy link
Contributor Author

@keewis maybe you can check the wording for the return values?

I am using persisted DaskArrays not sure that's a commonly used term? In the discussion #6837 you also say that it should cross-reference on the DaskArray docs so I added that as well

keewis
keewis reviewed Apr 2, 2024
View reviewed changes
xarray/core/dataarray.py Outdated Show resolved Hide resolved
xarray/core/dataarray.py Outdated Show resolved Hide resolved
xarray/core/dataarray.py Show resolved Hide resolved
xarray/core/dataarray.py Outdated Show resolved Hide resolved
xarray/core/dataset.py Outdated Show resolved Hide resolved
xarray/core/dataset.py Outdated Show resolved Hide resolved
saschahofmann and others added 6 commits April 2, 2024 16:11
@saschahofmann @keewis
Use new wording for persist
2ff82cb 
Co-authored-by: Justus Magin <keewis@users.noreply.github.com>
@saschahofmann @keewis
Update xarray/core/dataarray.py
d250ce7 
Co-authored-by: Justus Magin <keewis@users.noreply.github.com>
@saschahofmann @keewis
Apply suggestions from code review
26668b0 
Co-authored-by: Justus Magin <keewis@users.noreply.github.com>
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
744d385 
for more information, see https://pre-commit.ci
@saschahofmann
Add "all" to persist docstring
c8558a4
@saschahofmann
Merge branch 'update-persist-doc' of github.com:saschahofmann/xarray …
a2447e1 
…into update-persist-doc
max-sixty
max-sixty reviewed Apr 2, 2024
View reviewed changes
xarray/core/dataarray.py Outdated Show resolved Hide resolved
max-sixty
max-sixty reviewed Apr 2, 2024
View reviewed changes
xarray/core/dataset.py Outdated Show resolved Hide resolved
@max-sixty
Copy link
Collaborator

Thanks a lot @saschahofmann , this is a nice docs contribution.

I added a couple of optional suggestions to avoid some double negatives :)

@saschahofmann @max-sixty
Apply suggestions from code review
82a4926 
Co-authored-by: Maximilian Roos <5635139+max-sixty@users.noreply.github.com>
@max-sixty max-sixty added the plan to merge Final call for comments label Apr 2, 2024
@max-sixty max-sixty merged commit c5b90c5 into pydata:main Apr 2, 2024
33 checks passed
@saschahofmann saschahofmann deleted the update-persist-doc branch April 3, 2024 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keewis keewis keewis left review comments

@max-sixty max-sixty max-sixty left review comments

Assignees
No one assigned
Labels
plan to merge Final call for comments
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Is .persist in place or like .compute?
3 participants
@saschahofmann @max-sixty @keewis