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

MkDocs documentation for ReadTheDocs #61

Merged
merged 9 commits into from
Feb 11, 2025
Merged

Conversation

asiomchen
Copy link
Collaborator

This pull request includes several updates to the documentation, configuration files, and contribution guidelines. The most important changes include adding a new Read the Docs configuration file, updating the CONTRIBUTING.md file with new instructions, and modifying the .pre-commit-config.yaml file to exclude .ipynb files from certain checks.

Documentation Updates:

Documentation should be good to go, docstring can still be improved, but this is good starting point.
Last step is to deploy the docs will be to activate RTD build, once PR is merged

  • Added MkDocs documentation for API and notebooks
  • Added a new Read the Docs configuration file (.readthedocs.yaml) to set up the documentation build environment and process.
  • Moved notebooks and their .py files to to separate folders, so the ipynb-py pair can be paired automatically

Configuration Updates:

  • Modified .pre-commit-config.yaml to exclude .ipynb files from the pretty-format-json check.

Code Ownership:

  • Updated .github/CODEOWNERS to assign specific files to their respective owners.

Build and Testing:

  • Added new Makefile targets for syncing and running notebooks.

These changes aim to improve the documentation process, streamline the contribution workflow, and ensure proper code ownership and formatting.

@asiomchen asiomchen added the documentation Improvements or additions to documentation label Feb 9, 2025
@asiomchen asiomchen requested a review from EBjerrum February 9, 2025 12:02
Copy link
Owner

@EBjerrum EBjerrum left a comment

Choose a reason for hiding this comment

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

Looks very good. There's two minor things I stumbled upen when looking through the docs. I commented on the relevant files.

Some of the doscstrings will need a bit of TLC (e.g. ,SECFingerprintTransformer) but I guess it gets easier to spot and correct once we get this doc system up and running, so that can come over time.

Just a question: After things are set up, the docs will be live on mkdocs? then _doc_link_template and _doc_link_url can be implemented on the classes.

docs/api/scikit_mol.standarizer.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@asiomchen
Copy link
Collaborator Author

Yeah, it will be much more convenient to work on docstrings once the docs deployment is enabled.
And yes _doc_link_url is worth adding after we 100% have the live docs and base URL claimed.

The docs will be live on ReadTheDocs - I already tested deployment on my fork and it works fine.

So after the PR will be merged what is left is to import project to RTD. It is simple and mostly automatic process.
Btw, do you prefer to do it yourself or I can handle it?

@EBjerrum
Copy link
Owner

Sounds great. You are welcome to put it live on RTD, sound like you already know what to do. But, can I also get administrator rights to the deployment there?

@EBjerrum EBjerrum merged commit 7290b03 into EBjerrum:main Feb 11, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants