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

resolve inconsistency between lint.sh and setup.cfg #1509

Merged
merged 1 commit into from
Mar 17, 2023

Conversation

haifeng-jin
Copy link
Contributor

setup.cfg says 80 chars per line, but lint.sh says 200 chars per line. This PR resolves this inconsistency.

The inconsistency between lint.sh and setup.cfg is causing the IDEs to print false lint warnings, because IDEs use setup.cfg only.

Now 80 char line length is only ensured by black.
There are a lot of over length strings, docstrings, and comments.

Ideally, we should still ensure 80 char line length and use # noqa: E501 after the docstrings like the following.

class SomeClass:
    """My docstring.

    This is a line with over length.
    """  # noqa: E501

@LukeWood
Copy link
Contributor

Thanks for the PR! The reason we have this as of now is for links, i.e.

class MyLayer:
    """
    References:
         - https://arxiv.org/my/super/long/link/that/is/10233243232/characters/long/and/definitely/over/80
    """

Open to alternatives, but thats the history of this configuration!

@haifeng-jin
Copy link
Contributor Author

@LukeWood Thanks for the context!
To be clear, this PR does not change this configuration, but only move the configuration into setup.cfg to make it universal.

We can change it later when we have the resource to do it.

Copy link
Contributor

@ianstenbit ianstenbit left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Haifeng!

I've opened #1532 for the long-term solution of enforcing 80 universally and exempting docstrings using noqa

@ianstenbit ianstenbit merged commit 3f34429 into keras-team:master Mar 17, 2023
Anirban0011 added a commit to Anirban0011/keras-cv that referenced this pull request Mar 17, 2023
resolve inconsistency between lint.sh and setup.cfg (keras-team#1509)
@haifeng-jin haifeng-jin deleted the flake8 branch March 17, 2023 21:57
ghost pushed a commit to y-vectorfield/keras-cv that referenced this pull request Nov 16, 2023
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.

3 participants