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

Sphinx documentation in doc folder #13

Merged
merged 10 commits into from
Aug 17, 2023
Merged

Conversation

engrosamaali91
Copy link
Contributor

**Sphinix documentation **

  • All the source files have been added to the doc folder as per LaPy documentation
  • The mains files are configuration file i.e conf.py and index.rst which the main page of BrainPrint project

What steps I followed to generate .rst files

  1. Git clone BrainPrint repository
  2. Modified configuration file i.e conf.py, added relevant extension. (Some lines have been commented out such as sphinx_gallery_conf = {} as they were generating warnings and errors
  3. Added relevant links to links.inc file
  4. Generated rst files corresponding to python files and packages

cd docs
sphinix-apidoc -o (current directory) (source directory)

  1. This sphinix-apidoc creates these rst files

brainprint.cli.rst
brainprint.commands.rst
brainprint.rst
brainprint.utils.rst
brainprint.utils.tests.rst
modules.rst

  1. Then copied the brainprint.rst files details to the my toctree file i.e api-> index.rst->bp.rst
  2. Built html files using make html

Note: The source files are not getting linked with corresponding github source code due to errors in step_2

@m-reuter m-reuter changed the title Sphinix documentatoin in doc folder Sphinx documentation in doc folder Jul 7, 2023
@m-reuter
Copy link
Member

m-reuter commented Jul 7, 2023

Hi, I wonder why there is no numpydoc in the extensions? And why you put so many rst files manually instead of autogeneration? Compare to how it is done in LaPy (also templates are missing)?

@engrosamaali91
Copy link
Contributor Author

Hello, I believe the sphinx-API doc in step-4 reads the .py files with docstring which I why I could see numpy docstring in the source code. Sphinx api doc command creates as many rst files as many python packages are there in the file. In brinprint we have utilis, commands, brainprint class, asymmetry, and onemore module. So sphinx api created all the corresponding rst files automatically, then its up to us which rst file we would like to add to our project. This is another method of autogeneration of rst files which I used instead of the one we used in LapY project. The reason why I used this method is that I was getting loads of warnings and errors while building html page with the LaPy method. I configured the rest as per LaPy method.

@codecov
Copy link

codecov bot commented Jul 19, 2023

Codecov Report

Merging #13 (d34a723) into main (e74b3a3) will not change coverage.
Report is 1 commits behind head on main.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #13   +/-   ##
=======================================
  Coverage   27.19%   27.19%           
=======================================
  Files           8        8           
  Lines         353      353           
  Branches       43       43           
=======================================
  Hits           96       96           
  Misses        255      255           
  Partials        2        2           
Flag Coverage Δ
unittests 27.19% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@engrosamaali91
Copy link
Contributor Author

engrosamaali91 commented Jul 19, 2023

Sphinix documentation based on LaPy method bbbcc9d

All the source files have been added to the doc folder as per LaPy documentation
The mains files are placed under doc folder

What steps I followed to generate .rst files:

  • Git clone BrainPrint repository (Latest version)
  • Modified configuration file i.e conf.py, added relevant extension as per LaPy method.
  • Modified linkcode_resolve function in conf.py file as the existing lincode_resolve function was unable to generate corresponding github source code (Please refer to the conf.py file for changes made in the function)
  • Added template as per LaPy method
  • Added links for the brainprint project in index.rst file

In order to solve import error, I have used isort tool which automatically sorted conf.py file.
Steps:

  • pip install isort
  • isort --check-only conf.py It checks the file for unformatted imports
  • isort conf.py Sorted import errors
  • isort --diff conf.py It ensures whether the changes have been made

Note: BrainPrint API reference are splited into modules and brainprint classes for distinction. This can be changed as per requirements.

@engrosamaali91
Copy link
Contributor Author

In commit 9170585 the absolute path(local) from sphinx gallery configuration has been removed so that the document can be built on any version control systems like Git.

Please refer to line no. 234 of conf.py file.

@engrosamaali91
Copy link
Contributor Author

In commit f0122ea index.rst file has been modfied as per readme.me content.

Convert README.md file which was in markdown format to index.rst file which is in restructured text.

With direct parsing of readme.md file the sphinix is unable to recognize .md format and therefore it does not generate proper headings and cover entire long text in oneline. It also does not recognize clickable pypi image in the start.

@engrosamaali91
Copy link
Contributor Author

  • In commit f957a5e sphinix gallary has been commented out in conf.py file
  • In commit c12843f api referencing structure has been changed and placed under API reference only.

Comment on lines 1 to 10
@article{conformal_parameterization_2020,
author = {Choi, Gary P. T. and Leung-Liu, Yusan and Gu, Xianfeng and Lui, Lok Ming},
doi = {10.1137/19M125337X},
journal = {SIAM Journal on Imaging Sciences},
number = {3},
pages = {1049-1083},
title = {Parallelizable Global Conformal Parameterization of Simply-Connected Surfaces via Partial Welding},
volume = {13},
year = {2020}
}
Copy link
Member

Choose a reason for hiding this comment

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

This reference should be dropped as it is only relevant for Lapy and not Brainprint

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reference has been dropped 20a1154

@m-reuter
Copy link
Member

I wonder if we can find out a way to directly include the README.md rather than having a copy in rst . Currently we need to remember to edit both files if we change anything, which is not great.

@m-reuter
Copy link
Member

black fails on conf.py

doc/conf.py Outdated
anchor = ""

# Replace <gh_url> with the actual GitHub repository URL
gh_url = "https://github.com/Deep-MI/BrainPrint" # Replace with your repository URL
Copy link
Member

@m-reuter m-reuter Aug 15, 2023

Choose a reason for hiding this comment

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

gh_url is set as a variable in line 24, why is that not enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed gh_url in line 24 is enough. Line 178 has been commented out. 46c2200

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, please remove that line and comments so that the function is identical again to the one in lapy.

@engrosamaali91
Copy link
Contributor Author

I wonder if we can find out a way to directly include the README.md rather than having a copy in rst . Currently we need to remember to edit both files if we change anything, which is not great.

True, I actually tried including README.md file directly into index.rst file but .rst file did not properly render and import .md file content and it was not properly formatted as per readme.md file. I will look more into other methods to include README.md file.

@engrosamaali91
Copy link
Contributor Author

black fails on conf.py

conf.py file has been reformatted using black. 521ad6c

@m-reuter
Copy link
Member

Conf.py still fails in black. Maybe gh action uses a different version of black?

@engrosamaali91
Copy link
Contributor Author

Conf.py still fails in black. Maybe gh action uses a different version of black?

Hello,
The linkcode_resolve function is now identical to the one used in LaPy. I have now commented out the linkcode_resolve function on line 142 because the linkcode_resolve used in LaPy wasn't properly linking the corresponding source code on GitHub after building for BrainPrint. However, it seems that the linking is now working as expected using LaPy linkcode_resolve function.

Regarding the issue with conf.py failing in black, I am using the same version (23.7.0) of black as GitHub. Additionally, I've ensured that the conf.py adheres to the configuration defined in 'Brainprint/pyproject.toml' by running black --verbose conf.py. My local and GitHub Actions configurations are in sync." d34a723

@m-reuter m-reuter merged commit 713cb90 into Deep-MI:main Aug 17, 2023
27 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