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

Modifying docs to simplify the discussion of basic chemiscope usage #330

Merged
merged 15 commits into from
Mar 21, 2024

Conversation

ceriottm
Copy link
Contributor

@ceriottm ceriottm commented Mar 18, 2024

  • Move around some of the code section, separating more clearly the Python package and API
  • Make an examples section, ideally including (links to?) the generated chemiscopes

@ceriottm ceriottm requested review from rosecers and Luthaf March 19, 2024 15:06
@ceriottm
Copy link
Contributor Author

It's not perfect, but - as the branch name says - it's "better" ^_^

@Luthaf
Copy link
Contributor

Luthaf commented Mar 19, 2024

This looks good overall. I'll reduce the PNG file sizes before merging, but it's not urgent.

Should we split the example into multiple pages instead of having all of them one after the other on the same page?

Copy link
Collaborator

@rosecers rosecers left a comment

Choose a reason for hiding this comment

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

In general looks good, left a few q's and comments throughout, but nothing super serious

.. figure:: img/example_pca.png
:align: center

Trajectory data from a MD simulation of allyl alcohol, including
Copy link
Collaborator

Choose a reason for hiding this comment

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

This PCA is very sparse -- it might be good to add some description of what's going on, especially because it's the first example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I was thinking of adding a truly basic example, load-from-ase-and-visualize as the first example. Would that be fine with you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a "simple example" does it help?

docs/src/manual/input.rst Show resolved Hide resolved
python/examples/colors.py Show resolved Hide resolved
@ceriottm
Copy link
Contributor Author

ceriottm commented Mar 19, 2024

This looks good overall. I'll reduce the PNG file sizes before merging, but it's not urgent.

Should we split the example into multiple pages instead of having all of them one after the other on the same page?

I'm ambivalent. I do not want to have sub-headings in the menu (that's also why I use these weird raw HTML headings, thanks ChatGPT) but it's true that this is very long. Maybe we could use a style that automatically folds and unfolds the navbar? We could also have a list of examples at the top linking to the relevant section further down.

@rosecers
Copy link
Collaborator

rosecers commented Mar 19, 2024 via email

@ceriottm
Copy link
Contributor Author

@rosecers forgive me - I also touched the code, but I think that this falls within the "better docs"

@ceriottm
Copy link
Contributor Author

@Luthaf if you're happy with this (I also reduced the size of the images) can you also make a point release, so that the docs correspond to the version most users will see?

@Luthaf
Copy link
Contributor

Luthaf commented Mar 21, 2024

can you also make a point release, so that the docs correspond to the version most users will see?

We don't need a release for this, docs are built from main

@ceriottm
Copy link
Contributor Author

can you also make a point release, so that the docs correspond to the version most users will see?

We don't need a release for this, docs are built from main

I meant that since most users will install from pip, it'd be nice for them to get the quick_settings() from there. Seriously, this was one of the most serious "small hassle" in using CS so hopefully it'll help.

@Luthaf
Copy link
Contributor

Luthaf commented Mar 21, 2024

right, I commented before seeing you added a function. I'll do a release!

@Luthaf Luthaf merged commit 0054189 into main Mar 21, 2024
9 checks passed
@Luthaf Luthaf deleted the better-docs branch March 21, 2024 16:09
@Luthaf
Copy link
Contributor

Luthaf commented Mar 21, 2024

Released in 0.6.1 !

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.

3 participants