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

Add sybil examples to more files #1586

Merged
merged 27 commits into from
Jul 20, 2023
Merged

Add sybil examples to more files #1586

merged 27 commits into from
Jul 20, 2023

Conversation

joaander
Copy link
Member

Description

Add sybil examples to all *.py files in the hoomd source directory. Also reword some documentation for clarity and reduce the toctree depth.

Motivation and context

Provide users with code examples that are tested and known to execute without errors.

How has this been tested?

I executed sybil tests locally (serial CPU build).

Change log

No additional change log is needed over that in #1574.

Checklist:

@joaander joaander requested review from a team, b-butler and SchoeniPhlippsn and removed request for a team July 14, 2023 17:06
Copy link
Member

@b-butler b-butler left a comment

Choose a reason for hiding this comment

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

The PR looks good overall I just have a few small questions/comments.

hoomd/write/gsd.py Outdated Show resolved Hide resolved
hoomd/simulation.py Outdated Show resolved Hide resolved
hoomd/md/update.py Outdated Show resolved Hide resolved
hoomd/logging.py Show resolved Hide resolved
The example simulation had 0 particles on rank 0, leading to
(0,) shape arrays instead of the expected (0,3) shape array.
@joaander joaander added the validate Execute long running validation tests on pull requests label Jul 18, 2023
Co-authored-by: Brandon Butler <[email protected]>
Copy link
Contributor

@SchoeniPhlippsn SchoeniPhlippsn left a comment

Choose a reason for hiding this comment

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

PR looks good. Only some minor comments.

hoomd/box.py Outdated Show resolved Hide resolved
hoomd/box.py Show resolved Hide resolved
@@ -681,13 +655,13 @@ def categories(self):

@property
def string_categories(self):
"""`list` of `str`: A list of the string names of the allowed \
"""list[str]: A list of the string names of the allowed \
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"""list[str]: A list of the string names of the allowed \
"""`list` [`str`]: A list of the string names of the allowed \

Copy link
Member Author

Choose a reason for hiding this comment

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

These are parsed correctly as is. The napoleon engine in sphinx understands these type annotations and expands list and str to links. We need explicit links only in cases where we add additional text to the type e.g. "list [str], optional".

hoomd/simulation.py Outdated Show resolved Hide resolved
@@ -348,32 +363,74 @@ def set_snapshot(self, snapshot):

@property
def particle_types(self):
"""list[str]: List of all particle types in the simulation state."""
"""list[str]: List of all particle types in the simulation state.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"""list[str]: List of all particle types in the simulation state.
"""`list` [`str`]: List of all particle types in the simulation state.

Same for all the following

@joaander joaander mentioned this pull request Jul 20, 2023
3 tasks
Copy link
Member

@b-butler b-butler left a comment

Choose a reason for hiding this comment

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

LGTM.

@joaander joaander merged commit 4d39c3b into trunk-minor Jul 20, 2023
@joaander joaander deleted the more-sybil branch July 20, 2023 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
validate Execute long running validation tests on pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants