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

[Enhancement] Update PyPi version and Sphinx docs? #262

Open
multimeric opened this issue Feb 22, 2023 · 18 comments · May be fixed by #270
Open

[Enhancement] Update PyPi version and Sphinx docs? #262

multimeric opened this issue Feb 22, 2023 · 18 comments · May be fixed by #270

Comments

@multimeric
Copy link
Contributor

I note that firstly the PyPI version is 18.8.1.1: https://pypi.org/project/pyslurm/. Also the docs seem to be only for 17.11.0.15. This is a pity, because recent versions of PySlurm have added lots of nice improvements! pip install pyslurm users and also people who visit the website won't be aware of many of these.

Is there any way I can help with getting these updated? Happy to write a GitHub actions publish action, for instance.

@tazend
Copy link
Member

tazend commented Feb 23, 2023

Hi, yeah you are right, the docs and pypi versions definitely need to receive an update.

For the PyPi part, this approach seems to be quite nice. In that guide, this pypi github action is used.
Whenever a commit with a tag is pushed, it will publish the release to PyPi.
Whenever a normal, untagged commit is pushed, it will publish a release to PyPi-Test to have testing-builds

What do you guys think about that? @multimeric @giovtorres

Not sure how an automated workflow to keep the docs at pyslurm.github.io would look though. I noticed its also only possible to view the docs for one version. Perhaps another enhancement to implement on the pages would be to be able to select the pyslurm version for which you want to view the docs? Not sure however if thats possible.

@multimeric
Copy link
Contributor Author

Yes, I think a github actions type solution to both of these would be ideal.

For the docs versioning, it seems there are a few solutions:

  1. Use ReadTheDocs, which do this automatically: https://docs.readthedocs.io/en/latest/versions.html
  2. With Sphinx, use this plugin: https://github.com/Holzhaus/sphinx-multiversion
  3. Move to MkDocs (which is a bit prettier I think), and use https://squidfunk.github.io/mkdocs-material/setup/setting-up-versioning/

@tazend
Copy link
Member

tazend commented Feb 27, 2023

ReadTheDocs could definitely be a good option for hosting the docs.
As for building the docs, right now the project has been using Sphinx, but MkDocs also looks pretty promising. I'll definitely check it out.

If you would like to write a GitHub action for publishing a new release to PyPi, that would be great :)
I think for the Events that trigger the upload, we could for now just check if the commit is tagged (which should include both pushing a tagged commit aswell as tagging an existing commit on GitHub directly?)

@multimeric
Copy link
Contributor Author

Sounds good, that's a pretty common workflow (to publish on git tag).

If you make a decision about the doc build I'm happy to implement it too.

@giovtorres
Copy link
Member

I'm late here, but this is an area of the project that needed some love for a while. I had started to publish versions a few years ago, but never got it to be fully automated with Travis CI (at the time), and it just rotted :(

Definitely sounds like GitHub Actions is the way forward here for docs! Multi-version docs is also great for this particular project. I've used both Sphinx and Mkdocs in the past and I don't have a preference for either. I'd go with whatever is easier at this point.

Thanks @multimeric for leaning in here, and thanks @tazend for stewarding all these recent changes 🎉

@tazend
Copy link
Member

tazend commented Feb 28, 2023

Another thing which I'd think would be nice at least for the in-code readability of the documentation is to switch over from reStructuredText to something like the Google docstring format in the future. Its just way better in terms of readability. Theres also numpydoc which is nice too.

Both the google and numpy format are prettier in my opinion (personally prefer the google style). Both sphinx and mkdocstring support them (and I think mkdocstring uses google format by default), so that wouldn't be a problem, and its rather just for the documentation style in the codebase.

I've seen that there are tools like pyment which can help to automate the switch (not sure if it can also handle cython pyx files though)

@multimeric
Copy link
Contributor Author

True, but I think that's a more minor point since it doesn't impact users.

@multimeric
Copy link
Contributor Author

In terms of versioned docs, mkdocs is out (#268), and I've tried sphinx-multiversion, but it has some serious issues with Cython projects, because each time you build the full docs, it tries to recompile each release by cloning each tag into a separate repo and then trying to extract the docstrings from that. However it neglects to compile the Cython when it does so, and the docs are therefore incomplete. In any case, it would take hours to compile every version whenever we do a docs build. So that is kind of out as well. So I guess we drop the idea of versioned docs for the moment?

@multimeric multimeric linked a pull request Mar 7, 2023 that will close this issue
@tazend
Copy link
Member

tazend commented Mar 7, 2023

Hi @giovtorres

to automatically publish new releases to PyPi in the future, a PyPi API-Token needs to be added as a Github Secret to this repository. Would it be possible to give me access to add Secrets, push access for pyslurm.github.io (for updating the documentation) and access to PySlurm's PyPi? (my PyPi user is also tazend)

P.S. I've also tried to contact you regarding maintaining PySlurm via your gmail (hope thats ok) - would appreciate if you could have a look :)

@tazend
Copy link
Member

tazend commented Mar 7, 2023

@multimeric yeah I would drop the focus on multiversion for now - maybe we find another solution later on or perhaps switching from github pages to something like readthedocs that handles multiversion might be the way to go then.

@tazend
Copy link
Member

tazend commented Mar 7, 2023

@multimeric

Alright, I've played with Cython and mkdocs a bit more - maybe we can still actually use mkdocs.
I've discovered a cython option called binding, which, when set to True, will actually make introspection for functions and such correctly work and the embedsignature flag would not be need anymore.

This option must be enabled explicitly in Cython 0.29.x, but is enabled by default in Cython 3.0 onwards.
I've tried Cython 0.29.33 with binding=True and embedsignature=False, and in mkdocs it looks like this for example:

image

Good so far, however the default values for arguments in a function are not preserved in Cython <= 3.0. I've then tried to compile pyslurm with the latest Cython (3.0.0b1), which worked basically without errors (just had to fix something small in the slurm header.pxi, renaming all cpdef enum to cdef enum) - and everything looks correct:

image

One thing mkdocs still doesn't do is interpreting a property or cdef public attribute correct on a cdef class. This will then look like this:

image

Sphinx for example does it correct though.
Initially I thought that would be a dealbreaker for #224, which makes heavy use of properties, but the workaround for now would be to just not put a docstring on a property or cdef public variable (mkdocs then ignores them), and just document all of the attributes in the docstring of the class as it is usually done, which then shows up correctly.

So, the only thing that is needed would be to use the latest Cython e.g. 3.0.0b1. We could just lift the current restriction that is in place in the pyproject.toml, which excludes Cython <= 3. Or just install Cython 3 explicitly when it comes to building the docs.
Then embedsignature can be dropped, and binding is enabled by default anyway. Didn't see anything unexpected when using Cython 3 with the current main and #224, everything seems to work just like before - Only the compile time has definitely increased a bit.

What do you think? Having multiversion docs would then be viable again

@multimeric
Copy link
Contributor Author

Sounds good! If it's possible, it would be nice to only require a newer Cython for the docs (on our CI sever), and otherwise leave all the parameters as default. That should give us the best of both worlds. I saw the binding option when I was playing around with mkdocs but for some reason thought it came with a performance penalty?

@multimeric
Copy link
Contributor Author

Feel free to open a PR if you're branched off my branch so I can test your changes to support Cython 3.

@tazend
Copy link
Member

tazend commented Mar 17, 2023

ping @giovtorres - would appreciate if you could check out my comment here :)

@tazend
Copy link
Member

tazend commented May 5, 2023

Good news: The documentation has now been updated at https://pyslurm.github.io/ with multi-version support already enabled 🎉 (although at the moment only 23.2 is shown). There is probably still many things to improve, but its a start :)

@mrariden
Copy link

mrariden commented Dec 1, 2023

Was pypi update with tag push ever added? I still see the latest release on pypi is 18.8.1.1 from Oct 10, 2018.

Could I help with the GH actions revisions?

@CarlosAndreo
Copy link

Hello,

Is there any news about this thread? I would like to be able to install slurm using pip, but the version it installs is 18.8.1.1 (currently outdated): pyslurm

In any other case, is there any way I can install it in my Python project and start working with it?

@tazend
Copy link
Member

tazend commented Oct 28, 2024

Hi @CarlosAndreo,

you can just clone the repo and select a specific branch/tag, and then follow the instructions in the README. You just need to point pyslurm to your Slurm installation, and can then do pip install . inside the repo.

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 a pull request may close this issue.

5 participants