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

feat(template): Add API documentation to the template 2/4 (Sphinx) #124

Merged
merged 12 commits into from
Jun 17, 2023

Conversation

YurelyCamacho
Copy link
Collaborator

@YurelyCamacho YurelyCamacho commented Jun 12, 2023

Pull Request description

  1. Add the necessary dependencies in pyproject.toml and conf.py, to generate the API documentation with sphinx.
  2. Remove unusable file in sphinx (post_gen_project.py)
  3. Create and edit the file for automatic API documentation.
  4. Edit the index.rst with all the files

By @YurelyCamacho and @Anavelyz

@YurelyCamacho YurelyCamacho requested a review from xmnlab June 12, 2023 23:20
@@ -1 +1,4 @@
Introduction
Copy link
Member

Choose a reason for hiding this comment

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

I think that now I got the point you mentioned on discord.
that makes more sense to call this file to intro.rst
also please rename it https://github.com/osl-incubator/scicookie/pull/124/files#diff-e5704837157913d9457988b9450bc2f363e27663bd5e2ecd098e580a44e9adb9R11

Copy link
Member

@xmnlab xmnlab left a comment

Choose a reason for hiding this comment

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

thanks for working on that @YurelyCamacho
in general it looks good to me.
just added few comments here.
thanks!

Co-authored-by: Ivan Ogasawara <ivan.ogasawara@gmail.com>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@xmnlab We have several comments here:

  1. In line 4, we add the README directly, so that it appears as an introduction. When generating the documentation, sphinx does not recognise titles starting with #. The same happens when we create a link to an introduction page (where we use the readme.rst file already created). If you add ../README.md to the index, this link will not be generated. The same situation occurs if readme.rst has no title.
  2. There is a problem generating the contents of the example.ipynb notebook. It says that the title is not recognised and therefore it does not generate it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@xmnlab The issue here is whether to leave the readme as in line 4 or 11? And any suggestions on how to solve the problem of not recognising the headers (which are in md with #)?

Copy link
Member

Choose a reason for hiding this comment

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

ok give me until tomorrow to digest all of this and I will be back here tomorrow
thanks for your work here!

Copy link
Member

Choose a reason for hiding this comment

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

@YurelyCamacho, I am back here. I got the problem now. sorry for being late ...
I am investigating it now and I will be back here in a bit.

Copy link
Member

Choose a reason for hiding this comment

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

reading a bit this SO: https://stackoverflow.com/questions/46278683/include-my-markdown-readme-into-sphinx

it seems it has 3 options:

  1. using symlink https://stackoverflow.com/a/48931594
  2. creating a new file inside the docs folder that includes the README https://stackoverflow.com/a/68005314

not sure if it would be necessary to add a recommonmark as a (dev) dependency

Copy link
Collaborator Author

@YurelyCamacho YurelyCamacho Jun 14, 2023

Choose a reason for hiding this comment

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

@xmnlab We will try to see if it works for us and let you know, thanks

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@xmnlab Do you prefer the readme to appear on the first page (as an introduction) with the table of contents or in a link called introduction?
We have already resolved the issue of titles. What it still doesn't show us is the notebook information example.ipynb.

Copy link
Member

Choose a reason for hiding this comment

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

I don't have a strong opinion about that right now
so feel free to pick one that you think that would be better.
maybe soon we can get some feedback from some users and change it if it is necessary.

Copy link
Member

Choose a reason for hiding this comment

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

could you ping me tomorrow so we can see this issue with example.ipynb together?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

could you ping me tomorrow so we can see this issue with example.ipynb together?

All right, see you tomorrow

@YurelyCamacho YurelyCamacho requested a review from xmnlab June 13, 2023 00:36
@xmnlab
Copy link
Member

xmnlab commented Jun 15, 2023

@YurelyCamacho it seems that src/scicookie/{{cookiecutter.project_slug}}/pyproject.toml has a git conflict

@YurelyCamacho YurelyCamacho marked this pull request as ready for review June 16, 2023 19:27
@YurelyCamacho
Copy link
Collaborator Author

@xmnlab We made a few changes and locally all the documentation now shows up fine, with the readme as a link called Introduction and the notebook called Example.
On my machine, I had to install pandoc using the package manager. I put pandoc as a dependency in the pyproject.toml, but it gives an error with pandoc. What do you suggest?

@xmnlab
Copy link
Member

xmnlab commented Jun 16, 2023

@YurelyCamacho it seems that the problem is about the pandoc package from pypi: https://stackoverflow.com/questions/62398231/building-docs-fails-due-to-missing-pandoc

probably the way to go is to add pandoc just in the conda env.
as it is a dev dependency, that would be ok.

just not sure if the user decided to not use conda XD
but it would be a problem for the future!

let me know if that solutions works for you

@xmnlab
Copy link
Member

xmnlab commented Jun 16, 2023

@YurelyCamacho it seems we will need both packages, one installed by conda and the other one by poetry.
a friend pointed my this (from the documentation on pypi):

Pandoc – the general markup converter (and Haskell library) written by John MacFarlane – needs to be available. You may follow the official installation instructions or use conda:

$ conda install -c conda-forge pandoc
Then, install the latest stable version of the pandoc Python library with pip:

$ pip install --upgrade pandoc

@@ -7,3 +7,4 @@ dependencies:
- poetry
- nodejs # used by semantic-release
- shellcheck
- pandoc
Copy link
Collaborator

Choose a reason for hiding this comment

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

@xmnlab @YurelyCamacho I think it should be:

Suggested change
- pandoc
{% if cookiecutter.documentation_engine == 'sphinx' -%}
- pandoc
{% endif %}

Am I right?

Copy link
Member

@xmnlab xmnlab Jun 16, 2023

Choose a reason for hiding this comment

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

@YurelyCamacho could implement this idea @Anavelyz suggested?
please just to ensure to use the jinja tags correctly .. maybe:

{% if cookiecutter.documentation_engine == 'sphinx' -%}
  - pandoc
{%- endif %}

Copy link
Member

Choose a reason for hiding this comment

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

@YurelyCamacho maybe you can try this:

{%- if cookiecutter.documentation_engine == 'sphinx' %}
  - pandoc
{%- endif %}

btw, I recommend you to play with: http://jinja.quantprogramming.com/

for this case, you can use as value (yaml), this input:

cookiecutter:
  project_slug: test
  documentation_engine: sphinx

@xmnlab
Copy link
Member

xmnlab commented Jun 16, 2023

https://github.com/osl-incubator/scicookie/actions/runs/5294889705/jobs/9584695651?pr=124#step:5:355

@YurelyCamacho it seems that the tag jinja2 in the conda env file is not working properly yet with the fix-end-of-file pre-commit XD
ping me again when this PR is ready again for review :)

@YurelyCamacho
Copy link
Collaborator Author

@xmnlab Done

@xmnlab xmnlab merged commit e3b2baf into osl-incubator:main Jun 17, 2023
@xmnlab
Copy link
Member

xmnlab commented Jun 17, 2023

thanks @YurelyCamacho !

@xmnlab xmnlab changed the title feature(template): Add API documentation to the template 2/4 (Sphinx) feat(template): Add API documentation to the template 2/4 (Sphinx) Jun 17, 2023
@github-actions
Copy link

🎉 This PR is included in version 0.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants