scikit-learn-contrib / scikit-matter

A collection of scikit-learn compatible utilities that implement methods born out of the materials science and chemistry communities
https://scikit-matter.readthedocs.io/en/v0.2.0/
BSD 3-Clause "New" or "Revised" License
70 stars 18 forks source link

Cleanup rst files #179

Closed PicoCentauri closed 1 year ago

PicoCentauri commented 1 year ago

:books: Documentation preview :books:: https://scikit-matter--179.org.readthedocs.build/en/179/

PicoCentauri commented 1 year ago

Thanks for the review. You raised the right questions!

Thanks for the work. Just need some clarifications before merge: Did you use some rst file formatter for this?

So far not. I just used the rewrap plugin for vscode to do all the nasty formatting

We probably want to include such a formatter in the tox format testenv for developers.

Yes, we should add something. I already tooked around but I was not able to find the perfect plugin. We should include /flake8-docstrings. However, this will raise a lot of errors because it once the docstring in a very special format i.e docstrings for all public files, functions even in tests etc. We can disable these though.

As far as I understand black ignores rst files if you do not explicitly mention same. I don't have any experience with this, only found this https://github.com/LilSpazJoekp/docstrfmt

For rst files there are several linters and formatters but all of them have problems with the sphinx specific sections. I checked

Your looks quite promising though! I will have a look.

Convert README.md to README.rst (Preparations for later code sharing between REDME and docs)

Can you explain this a bit more? I am not sure what the reason is to change to rst.

Yes sure!. There are a lot of duplications in the README and the documentation pages on the one hand and on the other things which are in the README are missing in the documentation. If we have an rst file we can create markers and partly include text from the README in the documentation. This very useful but only works if the README is an rst file.

PicoCentauri commented 1 year ago

I checked docstrfmt. It is quite asgressive but looks good. The only thing is that it fails due to our citations I get an ValueError

ValueError: Unknown node type citation_reference!

I will investigate this, but maybe we do this after this PR?

PicoCentauri commented 1 year ago

Sphinx-lint seems to have a line width option. If this works this is enough for us I would say and we do not need an additional formatter. However, I tried it and it seems not to work... 😅

PicoCentauri commented 1 year ago

I fixed the options of the rst linter. The length of each line is now also checked.

agoscinski commented 1 year ago

but sphinx-lint is only checking and not formatting. My suggestion, we use for formatting docstrfmt and checking, it is only for the files bibliography.rst and selection.rst. We can ignore the formatting of the bibliography.rstand change the citation references in selection.rstto links. What do you think?

EDIT: And we open an issue on rstfmt to fix this https://github.com/dzhu/rstfmt

PicoCentauri commented 1 year ago

Yes, we can definitely go for docstrfmt. But, should we first merge this and do it an extra PR? We already have a lot of changes already.