Closed Roshan-Thomas closed 4 weeks ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 71.82%. Comparing base (
5194a88
) to head (af47f85
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Thanks, a few comments/requests before going into code review:
As indicated in the issue, some interface has changed since writing the tutorial, and I would like you to go through the code on the tutorial to make sure they run, and that the outputs are the same as what you get with current version of the code (if not, please update the outputs shown in tutorial). for example, this part is now changed to pattern = circuit.transpile().pattern
https://github.com/Roshan-Thomas/graphix/blob/3603c6cca103293bec4b0a6932def5ee9125b23f/docs/source/tutorial.rst?plain=1#L110C10-L110C34
this part of the issue, for tutorial page, does not seem to be incorporated enough? make the page easier to follow. Use pattern.draw_graph() visualization tool as appropriate
Also for introduction pages, to incorporate latest version of the code and make them easier to follow
.
did the readthedocs.org build succeed? can you make it public so we can check it?
Hey @shinich1, my apologies for getting back to you pretty late, I had a lot of uni work to do.
I have completed the required changes,
The build can be found here. I ran the build a while back and it still hasn't completed. It successfully installed graphix and then remain stuck there.
Let me know if there are any more changes needed to be made.
Hey @shinich1, my apologies for getting back to you pretty late, I had a lot of uni work to do.
I have completed the required changes,
- updated the code on the introduction and tutorial pages to reflect the latest changes to the codebase.
- made the tutorial page a lot easier to follow, and
- added graph visualizations on the tutorial page.
The build can be found here. I ran the build a while back and it still hasn't completed. It successfully installed graphix and then remain stuck there.
Let me know if there are any more changes needed to be made.
@Roshan-Thomas Thanks! the docs seems to have built fine (it takes time building all the examples
from the code). I have a few remaining requests before merging:
Hi @shinich1, I have added the following changes,
Note: The logo was something I was not able to enlarge it as the spinxawesome-theme
does not have that feature. I have searched in the documentation and there is no mention on how to increase the size of the logo. If you know of the method to do it, please let me know and I can implement it.
Hi @shinich1, I have added the following changes,
- removed 'graphix documentation' on top-left corner
- added code highlighting
- changed tutorial heading levels as per request
- updated the tutorial page to have simpler language as per request.
Note: The logo was something I was not able to enlarge it as the
spinxawesome-theme
does not have that feature. I have searched in the documentation and there is no mention on how to increase the size of the logo. If you know of the method to do it, please let me know and I can implement it.
@Roshan-Thomas Thanks a lot for looking into this. just a final request, would furo
theme allow you to enlarge the logo? could you try with this logo file?
e.g. quimb
docs has nicely-sized logo and they seem to use furo
theme.
@shinich1 I have updated the theme to furo
and got the logo (dark and light themed) to work.
Also, I had to place the dark
and light
logo in the _static
folder as furo
reads the images from the _static
folder only, and for some odd reason the file paths to the logo
folder was not working.
Hope this fixes all the issues and the changes required.
@shinich1 I have updated the theme to
furo
and got the logo (dark and light themed) to work.Also, I had to place the
dark
andlight
logo in the_static
folder asfuro
reads the images from the_static
folder only, and for some odd reason the file paths to thelogo
folder was not working.Hope this fixes all the issues and the changes required.
nice, it looks like the build failed - could you fix it? https://app.readthedocs.org/projects/graphix-uh/
fixed the issue, the furo
package was not present in the requirements.txt
. I have added it now (see last commit).
fixed the issue, the
furo
package was not present in therequirements.txt
. I have added it now (see last commit).
will check once the build completed. I see lint CI failing below, could you black -l 120
the conf.py
(see our guide)?
fixed the issue, the
furo
package was not present in therequirements.txt
. I have added it now (see last commit).will check once the build completed. I see lint CI failing below, could you
black -l 120
theconf.py
(see our guide)?
should I run the lint for any other files (other than conf.py
)?
@Roshan-Thomas all look good now - I will merge this once tests passed and I checked things again. btw, I'd appreciate if you could star the repo, if not done yet :)
@Roshan-Thomas all look good now - I will merge this once tests passed and I checked things again. btw, I'd appreciate if you could star the repo, if not done yet :)
Thank you!
Context: Unitary Hack task for updating the tutorial and documentation style.
Description of the change: The following changes are implemented in this PR
sphinxawesome_theme
and made sure the changes were reflected in all the pages.tutorial
was updated, and both required examples were added to it and deleted from the examples menu.Related issue: The related issue is #137.