search

Sm00thix / IKPLS

Fast CPU and GPU Python implementations of Improved Kernel PLS by Dayal and MacGregor (1997) and Shortcutting Cross-Validation by Engstrøm (2024).
https://ikpls.readthedocs.io/en/latest/
Apache License 2.0
10 stars 4 forks source link

contributing documentation improvements #10

Closed parmentelat closed 6 months ago

parmentelat commented 7 months ago

here are a few ideas for possible improvements on the contributing documentation

parmentelat commented 7 months ago

and oops I forgot to link the meta-issue https://github.com/openjournals/joss-reviews/issues/6533

Sm00thix commented 7 months ago

Hi @parmentelat. Thank you for the feedback and for your ideas.

I will update CONTRIBUTING.rst to reflect the use of poetry mentioned in your second point.

As for your first point, it was intentional to leave out CONTRIBUTING.rst from the documnetation on readthedocs. The thought behind this was that the documentation should only reflect how to use the software. However, I have now (currently only in the dev branch of the IKPLS repo) added a section "Contribute" to the README.rst that links to the CONTRIBUTING.rst. As the README.rst is shown on the frontpage of the readthedocs site, information regarding how to contribute will also be accesible through readthedocs (once I merge the dev and main branches). Are you satisfied with this solution or do you prefer something else?

parmentelat commented 7 months ago

hey there

yes, I was just for the material in that file to be made available under a human-readable form somehow , so that's fine !

Sm00thix commented 7 months ago

That's great, thank you! I will close this issue once I merge the dev and main branches.

Sm00thix commented 6 months ago

The changes have been merged to the main branch.

parmentelat commented 6 months ago

on second thought, I was looking at the documentation on readthedocs earlier today and found it quite hard to locate the link to the contributing doc, even though I knew it was there, so if there's any way to make this more visible it'd be great

Sm00thix commented 6 months ago

on second thought, I was looking at the documentation on readthedocs earlier today and found it quite hard to locate the link to the contributing doc, even though I knew it was there, so if there's any way to make this more visible it'd be great

Hi @parmentelat Thank you for the feedback. I am not quite sure how to make it more visible aside from moving the section up higher in the README.rst. However, I feel like the "Contributing" section belongs at the bottom of the page where it currently is. I justify this with the idea that all the other sections are more relevant for most users as I expect only a fraction of these actually want to contribute to the software.

If you have a concrete suggestion for making it more visible then I am happy to follow up on it. Otherwise, I urge you to consider the argument that a user willing to contribute will likely have read the entire README.rst and have found the "Contribute" section with the link to CONTRIBUTING.rst.

parmentelat commented 6 months ago

well, here's my user experience I had seen the CONTRIBUTING file in the repo; but because it's in rst and not in markdown I could not read it right away (I even went to the extent of trying to install vscode add-ons to get an inline preview; it took me 5 true minutes to realize that after 3 installed extensions it would no go through, not sure what exactly is broken in the stack, but that was not a workable path..) so at that point in order to see it plainly I could have thought of going to github; but that has not been my path instead I went to readthedocs; I must add that I am used to projects where the readthedocs doc does include this material as a plain section; and I could not find one....

so, maybe the simplest answer would just be to go for a markdown document instead; with that format anyone with vs-code can immediately read the rendered instructions without going through all this pain (and same goes for the README btw...) note that in the current state, going for markdown on this two files would have 0 drawback as far as I can see

as you have seen I did an attempt in this direction (see #16), so as far as contributing at least, this is an easy move for you if you agree on my rationale

ps. as an aside, having an immediate inline preview makes it also soooo much easier to modify the text..

parmentelat commented 6 months ago

I am not quite sure how to make it more visible aside from moving the section up higher in the README.rst If you have a concrete suggestion for making it more visible then I am happy to follow up on it.

I'll be posting a micro-PR shortly that will do that

Sm00thix commented 6 months ago

so, maybe the simplest answer would just be to go for a markdown document instead; with that format anyone with vs-code can immediately read the rendered instructions without going through all this pain (and same goes for the README btw...) note that in the current state, going for markdown on this two files would have 0 drawback as far as I can see

I can understand why you would think this. And, initially, when I started the repo, the README was indeed markdown. However, I remember having some issues with the PyPI webpage (https://pypi.org/project/ikpls/) having issues with this. I can not remember if the issue was something about rendering MathJax or correctly showing the references/citations which I have in the README. But if my memory serves me well, it was one of these two things that gave issues. However, with reStructuredText, the issue was no longer. So, I would very much like to keep the README in .rst format.

However, there is probably no drawback in using markdown for the CONTRIBUTING file. So I will change CONTRIBUTING to markdown.

Sm00thix commented 6 months ago

I am not quite sure how to make it more visible aside from moving the section up higher in the README.rst If you have a concrete suggestion for making it more visible then I am happy to follow up on it.

I'll be posting a micro-PR shortly that will do that

Thank you very much. I have accepted it :-)

Sm00thix commented 6 months ago

Hi @parmentelat

After having merged your PR everything worked fine. However, after I changed CONTRIBUTING.rst to CONTRIBUTING.md as you suggested, the docs seem to not be built correctly. I tried some things but I could not get it to work. Do you have a suggestion for getting your PR to work with CONTRIBUTING.md or should I just stick to CONTRIBUTING.rst?

parmentelat commented 6 months ago

Hey @Sm00thix

sphinx is able to cope with markdown sources, I remember having done that in a few projects, except that of course I can't remember howto off the top of my head I'll do a bit of digging, and will let you know how to do that it'd help if you gave the specific error that you are running into though

parmentelat commented 6 months ago

so, here's what I found from another project that I'd been maintaining at some point the doc in this project had been progressively converted from rst to md, so it should be a good starting point:

IIRC that's the gist of it; let me know if that's not enough information

Sm00thix commented 6 months ago

Hi @parmentelat,

Thank you very much for the resources. They were quite helpful and allowed me to get started and fairly easily change CONTRIBUTING to md.

However, with README it is a different story. I would have liked to be able to define a target with {#sometarget} and later reference it like [text linking to the target](#sometarget). Using the VSCode markdown preview, it seemed to work correctly with the link being active and jumping to the place of the definition when being clicked upon. It should also be correct according to the MyST documentation. However, navigating to docs/ and executing python -m sphinx -T -b html -d _build/doctrees -D language=en . html which is what readthedocs executes, I got warnings like IKPLS/README.md:18: WARNING: 'myst' cross-reference target not found: 'sometarget' [myst.xref_missing].

I only wanted the above functionality so that I could correctly refer to the 7 references I have in the README. I worked around the by having docs/conf.py include the line myst_heading_anchors = 2 which would at least allow me to reference sections. Then I could just refer to the newly added Reference section in the README which is almost as good as being able to refer to the individual references in the Reference section.

In conclusion I have changed README and CONTRIBUTING from rst to md. I was unsuccessful in being able to refer individually to each of my 7 references in README but worked around this by just refering to the References section instead of the individual reference. If you know how to solve the issue with references then that would be greatly appreciated. Otherwise, I am satisfied with the state of things now.

Sm00thix commented 6 months ago

On second thought: I have just published the updated documentation on PyPI. And the reference links [1]-[7] are all broken. Additionally the text for reference [7] is not rendering correctly at all.

With this in mind and considering my previous comment, I would like to go back to README.rst. I can keep CONTRIBUTING as .md without an issue. How do you feel about this? @parmentelat

Sm00thix commented 6 months ago

This is fixed since https://github.com/Sm00thix/IKPLS/commit/c1a795b3a59b80793f6896c1dc02b53820137a7d .