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

Fun edge case in D407 #5378

Closed
john-science opened this issue Jun 26, 2023 · 4 comments · Fixed by #5383
Closed

Fun edge case in D407 #5378

john-science opened this issue Jun 26, 2023 · 4 comments · Fixed by #5383
Assignees
Labels
docstring Related to docstring linting or formatting question Asking for support or clarification

Comments

@john-science
Copy link

Let's say I have a function in my code with a standard NumPy docstring:

def something_amazing(inputs):
    """
    Wow, this does stuff!
    
    Parameters
    ==========
    inputs: dict
        Oh boy, oh boy
    """
    return sum(inputs.keys())

This will return the ruff error:

λ ruff .
awesome_sauce.py:6:9: D407 [*] Missing dashed underline after section ("Parameters")
Found 1 error.

But when I do a ruff --fix ., the code now looks like this:

def something_amazing(inputs):
    """
    Wow, this does stuff!
    
    Parameters
    ----------
    ==========
    inputs: dict
        Oh boy, oh boy
    """
    return sum(inputs.keys())

This happened hundreds of times in our codebases, because someone thought dashes equal signs were the same.

Now, I'm not sure if you want to support finding and fixing this problem. But in an ideal world I wouldn't have to worry about ruff --fix breaking things.

So, just FYI.

@charliermarsh
Copy link
Member

Haha. When an issue has "Fun" in the title...

Do you think D407 should detect and replace the equals signs?

@charliermarsh charliermarsh added question Asking for support or clarification docstring Related to docstring linting or formatting labels Jun 26, 2023
@zanieb
Copy link
Member

zanieb commented Jun 26, 2023

If they're of a matching length it seems like a good user experience to remove the equal signs automatically.

@john-science
Copy link
Author

Do you think D407 should detect and replace the equals signs?

It would have helped my team. Luckily, someone didn't trust the auto --fix flag and reviewed my PR.

BUT, I'm willing to concede I don't have a wide view of the huge feature set that ruff wants to support. So it's up to ya'll.

I could take a whack at it, of course. But I haven't tried to contribute to ruff before, so there would be a long spin-up process.

@charliermarsh
Copy link
Member

All good, @dhruvmanila already finished it :)

charliermarsh pushed a commit that referenced this issue Jun 27, 2023
## Summary

Replace same length equal line with dash line in D407

Do we want to update the message and autofix title to reflect this
change?

## Test Plan

Added test cases for:
- Equal line length == dash line length
- Equal line length != dash line length

fixes: #5378
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docstring Related to docstring linting or formatting question Asking for support or clarification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants