Skip to content

Conversation

@bcalford
Copy link
Contributor

Adds doc string to all of the modules that have been switched from private to public.

@christian-oreilly Options_mixin.py is currently left without docstring. This file is generated by another and has a warning not to edit it directly. Docstring shouldn't mess with anything, though. Should I add the docstrings directly to that file, or should I find a way to add them when generated with the generate_options_mixin.py file?

Closes #109

@christian-oreilly
Copy link
Collaborator

@bcalford Have a look at #105 and its diff. The renaming of the modules and the addition of the docstring has been done there and it is ready to merge. I am just waiting for @hanayik to add a codecov token to IPyNiiVue so that the coverage reported is added to the PRs. So, I am afraid that some of the work you did here is a duplicate of what I did in #105. However, the docstring that I made in #105 are less detailed than those that you add or modify in this PR. I added just one-liners wherever they were missing just to get the CI to pass the tests. I suggest you leave this PR aside for now, wait for #105 to merge, and then rebase against the main. If it is blocking you, I can merge #105 now and figure out the codecov after... the coverage is added anyway, it is just the report that is not displayed because I cannot add the codecov token to the repo.

@bcalford
Copy link
Contributor Author

@christian-oreilly This PR isn’t blocking anything for me, so no rush in merging #105.

The doc strings put in there look nice. I will review any differences between the ones here and in #105 after it merges and then add the elaboration on any classes/functions I think may need it.

@christian-oreilly christian-oreilly marked this pull request as ready for review February 12, 2025 02:45
Copy link
Collaborator

@christian-oreilly christian-oreilly left a comment

Choose a reason for hiding this comment

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

I am good with the state of that PR. @bcalford you can merge if you don't want to add anything else to that PR. Please squash the commits during the merging.

@christian-oreilly christian-oreilly self-requested a review February 12, 2025 02:48
@christian-oreilly christian-oreilly merged commit 706c394 into main Feb 26, 2025
19 checks passed
@christian-oreilly christian-oreilly deleted the doc_style branch February 26, 2025 14:47
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 this pull request may close these issues.

Every public class, function, class method, and class attribute has a doc string following pydocstyle

3 participants