Improve job.data, project.data (H5Store) examples. #235

bdice · 2019-10-25T02:39:58Z

Description

I created this PR to start addressing this issue: glotzerlab/signac-docs#50

@klywang Do you have specific suggestions on how to improve this? (I don't think I've "solved" the issue yet, since I haven't provided explicit examples for job.data as requested.) Feel free to edit this PR directly.

Motivation and Context

glotzerlab/signac-docs#50

Types of Changes

Documentation update
Bug fix
New feature
Breaking change¹

¹The change breaks (or has the potential to break) existing functionality.

Checklist:

I am familiar with the Contributing Guidelines.
I agree with the terms of the Contributor Agreement.
My name is on the list of contributors.
My code follows the code style guideline of this project.
The changes introduced by this pull request are covered by existing or newly introduced tests.

If necessary:

I have updated the API documentation as part of the package doc-strings.
I have created a separate pull request to update the framework documentation on signac-docs and linked it here.
I have updated the changelog.

vyasr · 2019-11-06T22:47:37Z

@klywang any feedback on this PR?

codecov · 2019-11-08T13:30:08Z

Codecov Report

Merging #235 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #235   +/-   ##
=======================================
  Coverage   65.38%   65.38%           
=======================================
  Files          42       42           
  Lines        5697     5697           
=======================================
  Hits         3725     3725           
  Misses       1972     1972

Impacted Files	Coverage Δ
signac/contrib/job.py	`90.63% <ø> (ø)`	⬆️
signac/contrib/project.py	`90.88% <ø> (ø)`	⬆️
signac/core/h5store.py	`90.28% <ø> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a57549...7ab8b48. Read the comment docs.

csadorf · 2019-11-12T08:53:09Z

Why is this PR not assigned to anybody?

vyasr · 2019-11-16T05:20:27Z

This PR was in draft mode until recently. When moved out of draft mode, what is our policy for assigning a maintainer in charge? I'm guessing the bot didn't add any reviewers, so nobody assigned themself to the issue.

bdice · 2019-11-16T05:39:05Z

I assigned myself to this PR and requested review from @klywang since she originally reported the documentation challenges. I marked it as "ready for review" (no longer draft mode) in hopes that it would get reviewed by someone, but I suppose the bot didn't assign reviewers.

klywang · 2019-11-17T19:10:27Z

Is there a specific reason you one should use H5Store over job.data or project.data? If there isn't a good reason, it would help to have an short example that shows its usage with the context manager. For example:

with job.data
net_energy = job.data.net_energy[:]
Just enough so that someone who wants to use the functionality can glance at the docs and know how to use it.

bdice · 2019-11-18T04:42:52Z

@klywang The purpose of linking to H5Store is that the job.data is an H5Store. I was hesitant to duplicate examples for H5Store in job.data and project.data, just to keep the code organized (changes to H5Store would have to be reflected in several examples, not just job.data and project.data but also job.stores, etc. -- hard to know exactly where to draw the line). That said, I see why it would be user-friendly to have a snippet like that in the docs. I'll adapt your example and ping you for review.

csadorf · 2019-11-18T07:58:41Z

@klywang The purpose of linking to H5Store is that the job.data is an H5Store.

Also might be worthwhile to try to make that relationship more explicit in the documentation to avoid confusion.

klywang · 2019-11-19T19:49:13Z

@csadorf
Poor wording on my end. I meant, under what circumstances should a user use H5Store or job.data\ project.data? I thought the documentation clearly communicated the fact that these features were H5Stores. The primary source of confusion was that there were a lot of different ways to do a similar operation and it was difficult to understand which version I should use, especially before I knew how to use it.

csadorf · 2019-11-20T15:59:38Z

@csadorf
Poor wording on my end. I meant, under what circumstances should a user use H5Store or job.data\ project.data? I thought the documentation clearly communicated the fact that these features were H5Stores. The primary source of confusion was that there were a lot of different ways to do a similar operation and it was difficult to understand which version I should use, especially before I knew how to use it.

@klywang I see. So it's more about when to use job.data, project.data, or a direct instance of H5Store. Could you maybe propose a paragraph that would have helped you to understand that better/earlier?

bdice · 2019-11-20T16:09:59Z

(I would also include job.stores in that description if possible)

klywang · 2019-11-20T21:59:43Z

For job.data
job.data lets you read and write data associated with your job. It should be used for large array-like data, which can't be stored efficiently in the job document. For examples and usage, see [link for topic guide].

For Topic Guides > Jobs > Job Data Store
Some revised examples for the Topic Guide already exist, but I rewrote them to streamline the rest of these examples.

Writing data
To write data to a store:

job.data['velocities'] = np.ones([10, 3, 2])

Data may also be written in subgroups:

job.data['net_energy/frame_10'] = np.ones([10, 3])

Reading data
To access data as an attribute:

with job.data:
    velocities = job.data.velocities[:]
    net_energy = job.data.net_energy.frame_10[:]

To access data as a key:

with job.data:
    velocities = job.data['velocities'][:]
    net_energy = job.data['net_energy/frame_10'][:]

Other uses
To view keys associated with job.data or in a subgroup:

print(list(job.data.keys())
[Output]: ['velocities', 'net_energy']

print(list(job.data.net_energy.keys()))
[Output]: ['frame_10']

To iterate over keys in job.data or a subgroup:

for frame_n in job.data.net_energy.keys():
    with job.data:
        frame_energy = job.data.net_energy[frame_n][:]

Data written with job.data is stored in a file named signac_data.h5 in the associated job folder. For cases where job-associated data may be accessed from multiple sources at the same time or other instances where multiple files may be preferred to one large file, job.stores should be used instead of job.data.

For more information about supported data types and the file format, please see documentation for the H5Store.

Also, note for myself -- there is a typo in the H5Store doc that I will fix.
P.S. Thank you @vyasr @bdice for the input!

vyasr · 2019-11-25T15:14:18Z

@atravitz @b-butler I've requested some additional reviews on here, please read through @klywang's suggested improvements to our documentation in her most recent comment on this thread and provide some commentary. Thanks!

atravitz · 2019-11-26T02:28:56Z

As someone who has only just recently started using job.data, this is incredibly clear and helpful! One minor thing: it might be better to name frame_10 something else. The way it currently reads is that it's an alternate way of accessing a frame, rather than being a sub-key.

vyasr · 2019-12-04T16:17:44Z

@klywang could you incorporate @atravitz's suggestion and then add your snippet to the relevant places in the documentation (i.e. update topic guides, then add links in the docstrings to point to the topic guides instead of just to the H5StoreManager and related classes).

b-butler

@klywang suggestions LGTM with @atravitz's correction.

bdice · 2019-12-11T03:02:32Z

@klywang We'll be making a release on Friday. If you have a chance to make the documentation changes by then (we also have a sprint on Thursday), that would be great!

atravitz

LGTM!

bdice · 2019-12-13T03:32:46Z

It doesn't look like @klywang (or anyone) pushed any commits with the drafted text that is in her comment above. I'm confused about whether @b-butler and @atravitz meant to approve the file changes as they are currently (which is incomplete), or approve the concept of @klywang's changes. The changes @klywang suggested look great to me, but we need to ...actually implement them in the docs. 😕

I'll be making a release of signac tomorrow, could someone from (@glotzerlab/signac-committers, @klywang) take that on and copy-paste the snippets into the right places in the docs? If so, please reply on this PR. Otherwise I will try to apply the changes myself before making the release. It seems like we're generally in agreement over their intended content.

atravitz · 2019-12-13T03:51:32Z

I’ll take care of this before 11 am tomorrow (Friday).

…

---------- On Thu, Dec 12, 2019 at 10:32 PM, Bradley Dice <notifications@github.com> wrote: It doesn't look like @klywang <https://github.com/klywang> (or anyone) pushed any commits with the drafted text that is in her comment above <#235 (comment)>. I'm confused about whether @b-butler <https://github.com/b-butler> and @atravitz <https://github.com/atravitz> meant to approve the file changes *as they are currently* (which is incomplete), or approve the *concept of @klywang <https://github.com/klywang>'s changes*. The changes @klywang <https://github.com/klywang> suggested look great to me, but we need to ...actually implement them in the docs. 😕 I'll be making a release of signac tomorrow, *could someone take that on and copy-paste the snippets into the right places in the docs*? If so, please reply on this PR. Otherwise I will try to apply the changes myself before making the release. It seems like we're generally in agreement over their intended content. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#235?email_source=notifications&email_token=AHT6IXZT7O3ZN7GMOF7PFMTQYL657A5CNFSM4JE5YSUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGYZJ6Y#issuecomment-565286139>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHT6IXZL2HKBWSSEISPBQMDQYL657ANCNFSM4JE5YSUA> .

adding code snippets

b-butler · 2019-12-13T14:26:02Z

@bdice I meant to approve the suggestion. I should have been more clear about that. Otherwise, like you said the PR is incomplete.

atravitz · 2019-12-13T15:33:59Z

Update: @klywang is adding these in, then I'll review

vyasr · 2019-12-13T15:51:10Z

Thanks for the update @atravitz!

In different repository?

atravitz

Just a few fixes to make it more consistent with the other doc strings

signac/contrib/job.py

signac/contrib/project.py

klywang · 2019-12-13T18:44:09Z

Topic guide updates are here

klywang

I think we can wrap this up now! Everything looks good.

Point to H5Store for job.data, project.data examples.

35e204a

bdice changed the title ~~Point to H5Store for job.data, project.data examples.~~ Improve job.data, project.data (H5Store) examples. Oct 25, 2019

Merge branch 'master' into docs/h5store-examples

59d008f

bdice marked this pull request as ready for review November 8, 2019 13:26

bdice self-assigned this Nov 16, 2019

bdice requested a review from klywang November 16, 2019 05:37

vyasr mentioned this pull request Nov 18, 2019

Duplicate docstrings between related classes and functions #242

Closed

vyasr requested review from atravitz and b-butler November 25, 2019 15:13

vyasr assigned vyasr and unassigned bdice Dec 4, 2019

bdice added this to the v1.3.0 milestone Dec 4, 2019

b-butler approved these changes Dec 4, 2019

View reviewed changes

atravitz previously approved these changes Dec 11, 2019

View reviewed changes

Merge branch 'master' into docs/h5store-examples

b47d41e

bdice requested a review from a team as a code owner December 13, 2019 03:45

Edit doc string for job.data

1d7493f

klywang requested a review from a team as a code owner December 13, 2019 18:21

klywang added 3 commits December 13, 2019 13:22

Update project docstring

9d05760

Not sure how to link to signac-doc repo

29eec93

Not sure how to link topic guides

aa3dbfb

In different repository?

atravitz reviewed Dec 13, 2019

View reviewed changes

signac/contrib/job.py Outdated Show resolved Hide resolved

signac/contrib/project.py Outdated Show resolved Hide resolved

klywang added 2 commits December 13, 2019 13:46

Consistency with docstring job

cc1eda9

project docstring consistency

2a5d2fa

klywang approved these changes Dec 19, 2019

View reviewed changes

bdice added 5 commits December 19, 2019 14:47

Merge branch 'master' into docs/h5store-examples

87b231e

Add Project.stores to documentation rubric.

b0d0314

Add proper link to job.data.

c27ffdf

Add proper link, fix formatting and typo.

02877e8

Minor formatting updates.

7ab8b48

bdice merged commit 2c0d9e4 into master Dec 19, 2019

bdice deleted the docs/h5store-examples branch December 19, 2019 20:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve job.data, project.data (H5Store) examples. #235

Improve job.data, project.data (H5Store) examples. #235

bdice commented Oct 25, 2019

vyasr commented Nov 6, 2019

codecov bot commented Nov 8, 2019 •

edited

Loading

csadorf commented Nov 12, 2019

vyasr commented Nov 16, 2019

bdice commented Nov 16, 2019

klywang commented Nov 17, 2019

bdice commented Nov 18, 2019 •

edited

Loading

csadorf commented Nov 18, 2019

klywang commented Nov 19, 2019

csadorf commented Nov 20, 2019

bdice commented Nov 20, 2019

klywang commented Nov 20, 2019 •

edited

Loading

vyasr commented Nov 25, 2019

atravitz commented Nov 26, 2019

vyasr commented Dec 4, 2019

b-butler left a comment

bdice commented Dec 11, 2019

atravitz left a comment

bdice commented Dec 13, 2019 •

edited

Loading

atravitz commented Dec 13, 2019 via email

b-butler commented Dec 13, 2019

atravitz commented Dec 13, 2019

vyasr commented Dec 13, 2019

atravitz left a comment

klywang commented Dec 13, 2019

klywang left a comment

Improve job.data, project.data (H5Store) examples. #235

Improve job.data, project.data (H5Store) examples. #235

Conversation

bdice commented Oct 25, 2019

Description

Motivation and Context

Types of Changes

Checklist:

vyasr commented Nov 6, 2019

codecov bot commented Nov 8, 2019 • edited Loading

Codecov Report

csadorf commented Nov 12, 2019

vyasr commented Nov 16, 2019

bdice commented Nov 16, 2019

klywang commented Nov 17, 2019

bdice commented Nov 18, 2019 • edited Loading

csadorf commented Nov 18, 2019

klywang commented Nov 19, 2019

csadorf commented Nov 20, 2019

bdice commented Nov 20, 2019

klywang commented Nov 20, 2019 • edited Loading

vyasr commented Nov 25, 2019

atravitz commented Nov 26, 2019

vyasr commented Dec 4, 2019

b-butler left a comment

Choose a reason for hiding this comment

bdice commented Dec 11, 2019

atravitz left a comment

Choose a reason for hiding this comment

bdice commented Dec 13, 2019 • edited Loading

atravitz commented Dec 13, 2019 via email

b-butler commented Dec 13, 2019

atravitz commented Dec 13, 2019

vyasr commented Dec 13, 2019

atravitz left a comment

Choose a reason for hiding this comment

klywang commented Dec 13, 2019

klywang left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 8, 2019 •

edited

Loading

bdice commented Nov 18, 2019 •

edited

Loading

klywang commented Nov 20, 2019 •

edited

Loading

bdice commented Dec 13, 2019 •

edited

Loading