Improve snapshot api and document State #1079
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some developers discussed in #819 that they expect methods that make in-place modifications to return the object. This enables chaining of modifications.
Per the discussion in #819, users find it confusing that the snapshot property access returns copies and does not allow direct modification. Make these explicit methods so that the intent of the design is clear as is the performance implications. Also, prevent the type names from changing when restoring snapshots. Allowing the names to change would lead to parameters for one type applying to another.
- Both files now pass cspell checks - Make a few clarifying statements
Give users a convenient shortcut to replicate a simulation state.
Make it clear to the user that the box property gives a copy and that setting the box is a separate operation.
sphinx formats properties with short docstrings side by side in a flowing layout. The layout is difficult to read as it makes an unevenly spaced grid of blocks. With this change, the `State` properties all render in a single column format.
Snapshot only offers brief descriptions of the particle properties. State gives a complete description.
joaander
requested review from
a team,
b-butler and
yuanzhou0827
and removed request for
a team
for more information, see https://pre-commit.ci
bdice
approved these changes
Jul 28, 2021
Co-authored-by: Bradley Dice <[email protected]>
joaander
added
the
validate
b-butler
requested changes
Jul 30, 2021
Co-authored-by: Brandon Butler <[email protected]>
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Implement the snapshot API improvements discussed in #819 . Also apply the same changes to the
State.boxproperty and add detailed documentation toStateto explain all of the data it stores.Motivation and context
See the extensive discussion in #819.
Resolves #819
How has this been tested?
The deprecated APIS are still present and use the new APIs.
Change log
Checklist:
sphinx-doc/credits.rst) in the pull request source branch.