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 enforcing pydocstyle rule D213 in ruff. #40448

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

ferruzzi
Copy link
Contributor

@ferruzzi ferruzzi commented Jun 26, 2024

Currently we have rules D212 and D213 on the ruff ignore list in pyproject,toml. They are conflicting rules and I want to propose that we work towards enabling one of them. In a nutshell:

class Banana():
    """This is a D212-style docstring.

    Note that the summary line of a multi-line docstring is the first physical line, immediately 
    following the opening triple-quote.
    """
class Apple():
    """
    This is a D213-style docstring.

    Note that the opening triple-quote is followed by a newline in this multi-line 
    docstring, giving the mandatory (per D205, which we already enforce) 
    one-line summary its own line.
    """

Personally, I always use D213 style when I write multiline doc strings and I find it easier on the eyes. When I enabled it and ran static checks, ruff actually auto-fixed all instances so this is an easy one to enable if we want to.


Changes

  • pyproject.toml - Manually moved D213 from the exclude list to the run list and added the comments about conflicting rules.
  • Ran static checks locally and ruff automatically modified all other files.

Related: #10742


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

Copy link
Member

@jedcunningham jedcunningham left a comment

Choose a reason for hiding this comment

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

I did the same change locally with ruff and compared to this branch - no other changes in here.

@jedcunningham jedcunningham merged commit a62bd83 into apache:main Jun 27, 2024
70 checks passed
jedcunningham added a commit to astronomer/airflow that referenced this pull request Jun 27, 2024
These were added into main after apache#40448 branched off.
jedcunningham added a commit that referenced this pull request Jun 27, 2024
These were added into main after #40448 branched off.
@utkarsharma2 utkarsharma2 added the type:misc/internal Changelog: Misc changes that should appear in change log label Jul 1, 2024
@utkarsharma2 utkarsharma2 added this to the Airflow 2.10.0 milestone Jul 1, 2024
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Jul 26, 2024
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Jul 26, 2024
These were added into main after apache#40448 branched off.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:API Airflow's REST/HTTP API area:CLI area:db-migrations PRs with DB migration area:Executors-core LocalExecutor & SequentialExecutor area:Scheduler including HA (high availability) scheduler type:misc/internal Changelog: Misc changes that should appear in change log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants