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

Add ignore-signatures option to [SIMILARITIES] section #3619

Closed
jnsnow opened this issue May 13, 2020 · 7 comments
Closed

Add ignore-signatures option to [SIMILARITIES] section #3619

jnsnow opened this issue May 13, 2020 · 7 comments
Assignees
Labels
Enhancement ✨ Improvement to a component Good first issue Friendly and approachable by new contributors Help wanted 🙏 Outside help would be appreciated, good for new contributors

Comments

@jnsnow
Copy link

jnsnow commented May 13, 2020

Is your feature request related to a problem?

Please describe A clear and concise description of what the problem is.

When comparing code that uses multiple lines for function signatures, it's often the case that classes that implement or extend an interface will have several repeated lines for that interface signature.

e.g. consider a class implementing the __exit__ interface for a context manager:

class ArbitraryExample:
    def __exit__(self,
                 exc_type: Optional[Type[BaseException]],
                 exc_val: Optional[BaseException],
                 exc_tb: Optional[TracebackType]) -> None:
        pass

We're at four lines. If two modules in the project use a context manager, it will trigger the similarity checker.

Describe the solution you'd like

A clear and concise description of what you want to happen.

Implement a 'ignore-signature' option for the [SIMILARITIES] section, and have it default to yes.

(In same cases I imagine it might be a legitimate concern that you have many functions with extremely similar interfaces. In the case of special dunder methods at least, we are generally targeting an interface and have no control over the factoring. An ignore option like this might be best, since it seems hard to accurately determine which cases of repetition are fixable and which are not.)

qemu-deploy pushed a commit to qemu/qemu that referenced this issue Jun 1, 2020
mypy considers it incorrect to use `bool` to statically return false,
because it will assume that it could conceivably return True, and gives
different analysis in that case. Use a None return to achieve the same
effect, but make mypy happy.

Note: Pylint considers function signatures as code that might trip the
duplicate-code checker. I'd rather not disable this as it does not
trigger often in practice, so I'm disabling it as a one-off and filed a
change request; see pylint-dev/pylint#3619

Signed-off-by: John Snow <jsnow@redhat.com>
Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200514055403.18902-14-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
ehabkost pushed a commit to ehabkost/qemu-hacks that referenced this issue Sep 25, 2020
The pylint similarity checks cannot distinguish parameter lists from
other code; with the QAPISchemaVisitor interface having long lists of
parameters, these similarity checks fire off in a way that's difficult
to disable in a targeted way without littering the code with pylint
pragmas.

There is a change request filed to be able to ignore parameter lists,
see: pylint-dev/pylint#3619
elmarco pushed a commit to elmarco/qemu that referenced this issue Oct 27, 2020
The pylint similarity checks cannot distinguish parameter lists from
other code; with the QAPISchemaVisitor interface having long lists of
parameters, these similarity checks fire off in a way that's difficult
to disable in a targeted way without littering the code with pylint
pragmas.

There is a change request filed to be able to ignore parameter lists,
see: pylint-dev/pylint#3619
@Pierre-Sassoulas Pierre-Sassoulas added the Enhancement ✨ Improvement to a component label Feb 20, 2021
@Pierre-Sassoulas Pierre-Sassoulas changed the title Add ignore-signature to [SIMILARITIES] section Add ignore-signatures option to [SIMILARITIES] section Apr 14, 2021
@Pierre-Sassoulas Pierre-Sassoulas added Good first issue Friendly and approachable by new contributors Help wanted 🙏 Outside help would be appreciated, good for new contributors labels Apr 14, 2021
@Pierre-Sassoulas Pierre-Sassoulas pinned this issue Apr 23, 2021
@adityagupta1089
Copy link
Contributor

Let me know if I can help here.

@Pierre-Sassoulas
Copy link
Member

Hey @adityagupta1089 , no one is assigned to the issue so if you want to handle it I can assign it to you :)

@adityagupta1089
Copy link
Contributor

@Pierre-Sassoulas cool, I'll probably need some background or otherwise I can go through the code. Do you have some proposal for a solution or should I propose one?

@Pierre-Sassoulas
Copy link
Member

Nice, I assigned you to the issue 😄 There is already an option for the similarity checker for imports, this one will probably be pretty close to it. You could check that here: https://github.com/PyCQA/pylint/blob/master/pylint/checkers/similar.py#L53 .

@adityagupta1089
Copy link
Contributor

@Pierre-Sassoulas I think the PR was merged but did not trigger closure of issue. (I mentioned closes #3619 in description)

@Pierre-Sassoulas
Copy link
Member

Thanks for notifying me, cleaning these periodically after the fact is a full day of work :D

@jnsnow
Copy link
Author

jnsnow commented May 25, 2021

For my own record, this was addressed in PR #4474

(Thank you for implementing it!)

lucagalbu added a commit to lucagalbu/readabook that referenced this issue Dec 21, 2021
When having a base class and a derived class, pylint can complain
because some methods have the same signatures. In particular, this
happens if the function signature spans more than 4 rows, triggering
the similarity checker. This is the case for the function
lemmas_frequency.
I disable this pylint feature.

For more info: pylint-dev/pylint#3619
lucagalbu added a commit to lucagalbu/readabook that referenced this issue Jan 4, 2022
When having a base class and a derived class, pylint can complain
because some methods have the same signatures. In particular, this
happens if the function signature spans more than 4 rows, triggering
the similarity checker. This is the case for the function
lemmas_frequency.
I disable this pylint feature.

For more info: pylint-dev/pylint#3619
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component Good first issue Friendly and approachable by new contributors Help wanted 🙏 Outside help would be appreciated, good for new contributors
Projects
None yet
Development

No branches or pull requests

3 participants