jfcabana / omg_dosimetry

Optimized Multichannel Gafchromic Dosimetry
https://omg-dosimetry.readthedocs.io/en/latest/
MIT License
7 stars 2 forks source link

ReadTheDocs documentation update needed #19

Open LuisOlivaresJ opened 9 months ago

LuisOlivaresJ commented 9 months ago

Hi Jean,

The documentation you wrote in request #12 for each module, is not show in Rad The Docs. That is because Read the Docs uses the docstring from the package stored in PyPi (the latest omg version on PyPi is 1.4.1). You need to update omg_dosimetry on PyPi to see the added new changes.

jfcabana commented 9 months ago

Oh I see! Thanks for pointing that out. I find that managing versions on PyPi a bit cumbersome. Do you know if there is any way that package versions on PyPi be updated automatically to follow releases on GitHub?

jfcabana commented 9 months ago

Well, I also just saw that on the GitHub it says that latest release is 1.4.1. Not quite sure how how to handle releases properly :D image

jfcabana commented 9 months ago

Just found this guide that seems to be the solution I am looking for. Will try to set that up! https://packaging.python.org/en/latest/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows/

LuisOlivaresJ commented 9 months ago

I don´t know how to optimize it.

What I do with my packages is to follow this guide (but as you said, it is cumbersome).

1.- Change pyproject.py https://github.com/jfcabana/omg_dosimetry/blob/2100e90047bdb8aa886493497153097148ca01ad/pyproject.toml#L5-L7 2.- Build the package

python -m build

3.- Upload to PyPi

twine upload dist/*

I am going to study how "Actions" works, looks interesting.

jfcabana commented 9 months ago

Take a look at PR #20 , I think this should work. From my understanding, once this is merges on master, we should be able to push a tag on a commit, and the automated actions should build a release both on GitHub and on PyPi also. I'll wait for your approval before merging.

LuisOlivaresJ commented 9 months ago

Fire it. I suspect there will be issues associated with dependencies but let see what happens.

jfcabana commented 9 months ago

Well, I must have missed something cause it does not seem to be working. I pushed a commit with a tag v1.5.1, but it still shows up as 1.4.1 on pypi and github.

jfcabana commented 9 months ago

Ok so I have published the latest release as v1.5.1. It shows up correctly on PyPi, but for some reason the documentation on readthedocs does not seem to have updated. It shows up as 1.5.0 and does not contain my additions made in #12 .

LuisOlivaresJ commented 9 months ago

Nice job, I would like to make a pull request (as a test), just to try it :smiley:.

By the other hand, Readthedocs uses "Documentation" branch, so it will not refresh until a change is made in that branch. Now it is updated.

jfcabana commented 9 months ago

Ok, I just found out why the actions were not working. Actions were disabled in the repository settings! image

jfcabana commented 9 months ago

Working now! image

LuisOlivaresJ commented 9 months ago

Sorry for the mess with TestPiPy. Do you get an email every time a run to testPyPi failed? I going to search how to exclude the "Documentation" branch.

Seems something like

on: push: branches:

jfcabana commented 9 months ago

No I haven't receive anything about testpypi. I did receive an approval request for deployment on PyPi. Did you push a new tag on master or was it the pull request from Documentation?

LuisOlivaresJ commented 9 months ago

It was from branch Documentation, #29. I went to Actions tab, look for the failed run and cliked on the "Re-run job" button, after you had removed the environment protection rule on PyPi. Captura

LuisOlivaresJ commented 9 months ago

Hi Jean, I made a tagged commit to Documentation branch, and it activated the workflow to publish on PyPi. I think we need to filter so only action to publish on PyPi get active when main branch have been changed. It will allow you to see and approve (if it's the case) the changes, before publishing to PyPi. Let my try to suggest a change to publish-to-test-pypi.yml file.

LuisOlivaresJ commented 9 months ago

I found this guide.

Basically, the publish to PyPi work could be activated only if a pull request closes and the pull request was also merged. What do you think? Would you like me to try to change to publish-to-test-pypi.yml file?

jfcabana commented 9 months ago

Yes I think that would be the way to go. It might not even need to be related to a tag push I think. Any pull request that is merged on the master branch should trigger the publication to pypi.

LuisOlivaresJ commented 9 months ago

Do you like the added content to ReadTheDocs calibration page? Feel free to change whatever you want or tell me if something needs to be rearranged.

jfcabana commented 9 months ago

I really like what you did with the documentation! This is exactly what this project needed to be usable and understandable. Really good job!

petruong commented 7 months ago

Hello there Luis! Thanks a lot for setting up documentation for the project. Would have been real useful when I first diving into the lines of code/functions years ago, hah.

Anyways, with the recent pull request merge from the quality of life advancements made (2023-12-15), does the documentation need to be manually updated or is it based on the script commentary/description?

LuisOlivaresJ commented 7 months ago

Hi Peter,

Thank you, but all the work has been done by Jean. By this moment, my contribution to omg_dosimetry has been the ReadTheDocs page (as a self-learning path to get experience with OMG).

The documentation on ReadTheDocs is build from these files: docs/source/calibration.rst, tiff2dose.rst and analysis.rst. These files should be updated manually (actually the last two are still in progress). Additionally, ReadTheDocs uses the docstring (the string directly below a class, function, etc. with triple-double quote) inside the modules (calibration.py, tiff2dose.py and analysis.py). For example, what you see here https://github.com/jfcabana/omg_dosimetry/blob/0f615c4418c0bd4f1f91c9e76c70f9f3b56f7aa3/src/omg_dosimetry/calibration.py#L50-L59 is the information that will be displayed on the corresponding API section.

To update the changes made on GitHug into PyPi (where actually ReadTheDocs reads the docstrings), there is an Action. Any pull request that is merged on the main branch should automatically trigger the publication to PyPi, but for some reason it did not get active. Let me try to know why.