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

Enable ruff rules #992

Merged
merged 15 commits into from
Jan 2, 2025
Merged

Enable ruff rules #992

merged 15 commits into from
Jan 2, 2025

Conversation

MarcSkovMadsen
Copy link
Collaborator

@MarcSkovMadsen MarcSkovMadsen commented Dec 25, 2024

This PR Enables the docstring rules:

  • D104: Checks for undocumented public package definitions.
  • D107: Checks for public init method definitions that are missing docstrings.
  • D400: First line should end with a period.
  • D401: First line of docstring should be in imperative mood.

This is just a step on the way to improving the content of the docstrings - especially for .rx and .param methods. But first I would like Ruff docstring support to be able to help me.

@MarcSkovMadsen MarcSkovMadsen marked this pull request as ready for review December 25, 2024 06:14
@MarcSkovMadsen MarcSkovMadsen requested review from maximlt and hoxbro and removed request for maximlt December 25, 2024 06:15
Copy link

codecov bot commented Dec 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.25%. Comparing base (f4c0041) to head (696a2d0).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #992   +/-   ##
=======================================
  Coverage   87.25%   87.25%           
=======================================
  Files           9        9           
  Lines        4928     4928           
=======================================
  Hits         4300     4300           
  Misses        628      628           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@maximlt maximlt left a comment

Choose a reason for hiding this comment

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

@MarcSkovMadsen why is there a mix of Google and Numpy styles?

    def __init__(self,lhs,rhs,operator,reverse=False,**args):
        """Initialize a BinaryOperator with operands, an operator, and optional arguments.

        Args:
            lhs: The left-hand side operand, which can be a NumberGenerator or a number.
            rhs: The right-hand side operand, which can be a NumberGenerator or a number.
            operator (Callable): The binary operator to apply to the operands.
            reverse (bool, optional): If `True`, swaps the left and right operands. Defaults to `False`.
            **args: Optional keyword arguments to pass to the operator when it is called.

        Notes
        -----
            It is currently not possible to set parameters in the superclass during
            initialization because `**args` is used by this class itself.

For this PR, and the others you have in preparation, I'm curious and would like to know more about your process. Do you sometimes use an LLM? Or just look at the current docstrings + doc + code ?

@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented Dec 30, 2024

Its a mistake. I'm so used to Google docstring style. I will fix it. I dont Think its an issue in other PRs.

For this PR I dont remember using a llm a lot as I expected simple to fix issues.

For the other PRs I used an llm. I started by explaining I'm updating docstring of HoloViz Param which is using numpy docstring style. I then copy (parts of) the class, function and/ or the user guide to give it context and then ask to Update the docstring. I then copy the proposed docstring back and start reviewing it. I compare to the original. Often i remove repeated text, add a Documentation section or change the examples. I've run all the examples in Ipython. Sometimes I copy the code back to the llm and ask it to improve it further.

@MarcSkovMadsen
Copy link
Collaborator Author

Thanks for fixing the Google Docstring style. As far as I can see the failing macos tests is due to some import change by Philipp.

Let me know if there is more I should do here. Thanks.

@philippjfr
Copy link
Member

As far as I can see the failing macos tests is due to some import change by Philipp.

It's not, the conda-forge build chain broke pyarrow via libprotobuff and libabseil. Have to wait until they fix it.

@philippjfr philippjfr merged commit b5a6630 into main Jan 2, 2025
16 checks passed
@philippjfr philippjfr deleted the enhancement/docstring-rules branch January 2, 2025 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants