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

Curate AtomGrid construction examples #196

Merged
merged 4 commits into from
Dec 24, 2023

Conversation

marco-2023
Copy link
Collaborator

  • Removed AtomGrid construction example from docstrings.
  • Created a Jupyter notebook with revised examples.
  • Integrated the notebook into the Sphinx documentation.

- Removed AtomGrid construction example from docstrings.
- Created a Jupyter notebook with revised examples.
- Integrated the notebook into the Sphinx documentation.
Copy link
Member

@FarnazH FarnazH left a comment

Choose a reason for hiding this comment

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

@marco-2023 thanks for your PR. The content was great showcasing various ways to construct atomic grids. The examples/text were clear and well-described.

I shared my revisions in fd34341. Most of the changes are as described in #202 (review). Additionally,

  1. Can you clarify (add a comment) to describe where the radius = 2.418849439520986 was taken from?
  2. The medium and fine examples/plots of AtormGrid.from_preset were indistinguishable, so I changed the element for one from Li to H. I also combined the example showing the rotate keyword, so that different grids can be easily compared. The example is tighter now.
  3. Some of the print statements were long requiring horizontal scrolling, so I changed the print format to make it easier for the users to change/understand things.

@FarnazH FarnazH force-pushed the atom_grid_construction_example branch from ccfc727 to fd34341 Compare November 18, 2023 22:23
Copy link
Member

@FarnazH FarnazH left a comment

Choose a reason for hiding this comment

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

Thanks @marco-2023.

@FarnazH FarnazH merged commit 0e8e0bc into theochem:master Dec 24, 2023
7 checks passed
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.

2 participants