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

docs(python): Address ignored Ruff doc rules #9919

Merged
merged 4 commits into from
Jul 17, 2023

Conversation

zundertj
Copy link
Collaborator

D101, D102 & D103 are all useful rules, so have added docstrings where possible. On some internal utils, I have marked private and/or used a noqa. This should stop us from adding undocumented public functions.

D100 & D104: we are not using module and package docstrings at all, is this something we want/need? As it is in the list of "remove errors" in pyproject.toml.

D105: we should probably do this, but there are a lot here.

D101, D102 & D103 are all useful rules, so have added docstrings where possible. On some internal utils, I have marked private and/or used a noqa. This should stop us from adding undocumented public functions.

D100 & D104: we are not using module and package docstrings at all, is this something we want/need? As it is in the list of "remove errors" in pyproject.toml.

D105: we should probably do this, but there are a lot here.
@github-actions github-actions bot added the internal An internal refactor or improvement label Jul 16, 2023
zundertj added 2 commits July 16, 2023 10:35
It seems we are not exactly sticking to FrozenSet, as we return a FrozenSet of DataTypeGroup, but take in different types in this constructor.
@mcrumiller
Copy link
Contributor

D100 & D104: we are not using module and package docstrings at all, is this something we want/need?

This is something that is super helpful to people like me, who come from the outside and have to just dive into the code base when trying to help out with not much guidance from docstrings. This is especially true on the rust side, where barely any of the modules have any top-level documentation and there are way more files, and I often find myself floundering to find out what each module is responsible for.

I'd be happy to help on this one (on python side at least). I'm one of those weirdos that sort of enjoys doing documentation.

@zundertj
Copy link
Collaborator Author

Just to be sure, addressing D100, D104 & D105 would be a separate PR.

Copy link
Contributor

@stinodego stinodego left a comment

Choose a reason for hiding this comment

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

Nice addition! Just one nitpicky thing, then we can go ahead and merge this.

This is something that is super helpful to people like me

Indeed, I think we should add module docstrings. I have done this in a few places, but many still missing. Feel free to add some!

py-polars/polars/expr/datetime.py Outdated Show resolved Hide resolved
@stinodego stinodego changed the title chore,doc(python): Address ignored Ruff doc rules chore(python): Address ignored Ruff doc rules Jul 16, 2023
@stinodego stinodego added the documentation Improvements or additions to documentation label Jul 16, 2023
@github-actions github-actions bot added the python Related to Python Polars label Jul 16, 2023
@stinodego stinodego changed the title chore(python): Address ignored Ruff doc rules docs(python): Address ignored Ruff doc rules Jul 16, 2023
@stinodego stinodego removed the internal An internal refactor or improvement label Jul 16, 2023
Copy link
Contributor

@stinodego stinodego left a comment

Choose a reason for hiding this comment

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

Great!

@stinodego stinodego merged commit 6b0bacc into pola-rs:main Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation python Related to Python Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants