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

docs: Restructure documentation #358

Merged
merged 1 commit into from
Oct 28, 2024
Merged

docs: Restructure documentation #358

merged 1 commit into from
Oct 28, 2024

Conversation

Kolarovszki
Copy link
Collaborator

@Kolarovszki Kolarovszki commented Oct 14, 2024

The documentation got restructured, and cards have been added with sphinx-design.

Also, several fixes got issued:

  • The code blocks didn't render well in the ipython notebooks, and to fix this, the IPython.sphinxext.ipython_console_highlighting is added to the extension list in conf.py.
  • Building on ReadtheDocs got fixed by adding the CMake build in a post_install job.
  • The original documentation URLs got modified to the new one.
  • Some tutorials got updated.

Copy link

codecov bot commented Oct 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.35%. Comparing base (704a6cc) to head (eeb40d6).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #358   +/-   ##
=======================================
  Coverage   97.35%   97.35%           
=======================================
  Files          72       72           
  Lines        3818     3821    +3     
=======================================
+ Hits         3717     3720    +3     
  Misses        101      101           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@antalszava antalszava left a comment

Choose a reason for hiding this comment

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

Overall looks good! 👍 I think it's a nice improvement. :)

I've just left a couple of minor comments.

@@ -13,6 +13,14 @@
# See the License for the specific language governing permissions and
# limitations under the License.

"""
CVQNN Module
============
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: how come = is used for underlining the title here, but - is used in the subsequent files?

e.g.,

Channels
--------

:link: separating-programs
:link-type: doc

Separate and compose Piquasso programs.
Copy link
Contributor

Choose a reason for hiding this comment

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

The title of this card matches the description: either the title could be shorter or the description could be more elaborate.


One could easily install Piquasso with the following command:
.. grid-item-card:: Installation
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these cards are great! Is there a way to set a smaller font size for the description and/or larger one for the titles? Right now the titles and the description have the same font size and that can be visually a bit confusing imho.

image

E.g., if the text: "Instructions on the installation of the Piquasso package" is 2-4 sizes smaller than the title "Installation", then it's easier to distinguish them and the reader can make a quick choice more easily, if they want to read the description or not.

docs/index.rst Outdated
Comment on lines 9 to 10
Piquasso is an open source Python package, which allows you to simulate a photonic
quantum computer. One could use Gaussian or Fock state corresponding to different
representations to run simulations.
quantum computer.
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: this one is a very important sentence, it's like the "mission statement" of the project or at least the one-sentence summary of it. Some (not too over the top) extra formatting could be nice visually.

Comment on lines 9 to 17
jobs:
post_install:
- pip install cmake pybind11[global]
- cmake -B build -DCMAKE_INSTALL_PREFIX=$(pwd)
- cmake --build build
- cmake --install build
Copy link
Contributor

Choose a reason for hiding this comment

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

imho it can be worth leaving a comment here on what the purpose of this step is, e.g., "These steps are required for building the documentation on ReadtheDocs".

The documentation got restructured, and cards have been added with
`sphinx-design`.

Also, several fixes got issued:
- The code blocks didn't render well in the ipython notebooks, and to fix this,
  the `IPython.sphinxext.ipython_console_highlighting` is added to the
  extension list in `conf.py`.
- Building on ReadtheDocs got fixed by adding the CMake build in
  a `post_install` job.
- The original documentation URLs got modified to the new one.
- Some tutorials got updated.
@Kolarovszki Kolarovszki merged commit 5a58661 into main Oct 28, 2024
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