In https://github.com/pandas-dev/pandas/pull/48759/files we're introduced pylint, but have turned off its warnings as there's a lot of them:
https://github.com/pandas-dev/pandas/blob/d719840e5a2639babab3b4646b11a111547c518f/pyproject.toml#L36-L67
Task here is:
- pick one of the above warnings / errors, and remove it from
pyproject.toml - run
pre-commit run pylint --hook-stage manual --all-files. If this already passes, skip to step 4. - fixup any warnings that result
- if
pre-commit run pylint --hook-stage manual --all-filespasses, then stage and commit your changes and open a pull request
If you believe a warning is a false-positive, then it's OK to ignore it in-line, e.g.
df.rename(id, mapper=id) # pylint: disable=redundant-keyword-arg
Please comment here which pylint warning you'll work on before starting so we don't duplicate work. No need to ask for permission to work on this, and no need to comment "take" as multiple people can work on this concurrently
NOTE: please do not write "closes #48855" in your PR
checks we probably can't turn off: - too-many-function-args - unsubscriptable-object - unsupported-assignment-operation - no-name-in-module - no-member - import-error - not-an-iterable - invalid-unary-operator-type - invalid-name - wrong-import-order (and anything else related with imports) - unexpected-keyword-argument - use-implicit-booleaness-not-len - use-implicit-booleaness-not-comparison - comparison-with-itself - no-else-* - line-too-long - overridden-final-method - singleton-comparison - unsupported-membership-test - pointless-statement - broad-except - undefined-variable
If you're not sure whether a check should fit into the above list, please do ask
Comment From: shivamurali
Hi there! I'll get to work on the "used-before-assignment" warning and remove it from the pyproject.toml file.
EDIT: PR went stale, anyone else is welcome to work on this
Comment From: Exoutia
Hi i am new to open source and i want to contribute to this project by fixing this but i cant seem to run pylint with --all-files option am i doing something wrong. or do i need to do something before hand beside making a branch and cloning the repo, can someone help me
Comment From: MarcoGorelli
Hi @Exoutia - please show your output
The command is pre-commit run pylint --all-files
Comment From: Exoutia
Hi @Exoutia - please show your output
The command is
pre-commit run pylint --all-files
Thanks, now it ran some problems has happened during installation of pre-commit i again installed it and it got fixed
Comment From: slackline
I'll work on removing import-error.
Comment From: gulyapulya
Hi! I am also new to the open source, but I want to contribute, I will work on the "no-method-argument".
Comment From: kostyafarber
Hi! I'll work on inherit-non-class.
Comment From: soumilbaldota
Hi, I will take bad-super-call warning, I have added the pull request above.
Comment From: FloHofstetter
Hi, I will take function-redefined warning.
Comment From: rokaicker
Hi ! I'm new to open source, but I'd like to tackle the too-many-function-args!
Comment From: MarcoGorelli
thanks @rokaicker - that one's probably too hard to turn off for now, without going through a deprecation cycle, but there's lot's of others to choose
I've added this one to the description as one that's probably not possible to turn off for now
Comment From: jetale
working on "unsupported-assignment-operation"
Comment From: Exoutia
i am working on "no-self-argument"
Comment From: shalearkane
I am working on "unexpected-keyword-arg"
Comment From: roadswitcher
I am working on no-name-in-module
Comment From: Leviob
Hi, I would like to work on "used-before-assignment," but I'm aware it was already claimed by @shivamurali. Can I submit my own PR for it? Thanks.
Comment From: joaomarcoscrs
I'll work on the not-callable
Comment From: rAwsam
Hi! I am also new to the open source, but I want to contribute, I will work on the "undefined-variable"
Comment From: TejasCreative
I'll work on no-member!
Comment From: sakalmon
I'll work on unsubscriptable-object
Comment From: sakalmon
I'll work on
unsubscriptable-object
I'm new to open source but I removed the above line and got numerous errors. All of them were "EE1136 - unsubscriptable (unsubscriptable-object)".
Pylint documentation suggests it may be because there is no __get__item method or __class_getitem__ for the class. I believe this is a false positive and the error will have to remain disabled.
Comment From: MarcoGorelli
thanks for looking into this @sakalmon - that's fine, I'll add that to the list of ones we can't turn off
Comment From: sakalmon
@MarcoGorelli No worries, happy to help!
Comment From: roadswitcher
I am working on
no-name-in-module
Disabling this resulted in a substantial number of E0611s that are tied to referencing calls in underlying C implementation within the pandas._libs/* directories. Pylint documentation for other errors implies that there are flags that could be invoked in order to make pylint start building AST trees from the underlying C modules, but that's probably out of scope for this issue.
As with @sakalmon's investigation of unscriptable-object, I think that no-name-in-module need to remain.
UPDATE: Finished my checks so that I'd have an Actual Number To Report: 174 false positives across 126 separate source files. I think that's past the point of "leave the warning turned off" and well into the realm of "didn't I have anything better to do with my time last evening".
Given how often Pandas invokes/depends on underlying C implementations for various things, I'm surprised it wasn't more.
I still think it's probably not something you can turn off at this time, but if you want those 126 changed files in a PR, @MarcoGorelli , let me know.
Comment From: TejasCreative
I'll work on
no-member!
Looks like no-member can't be turned off. After removing it, A wide variety of E1101: Instance of xyz has no abc member. Looks like this is a false positive for object members that are created dynamically, but exist at the time they are accessed.
Comment From: techrajdeep
I am looking for an issue for my first contribution . Can you help me here.
Comment From: slackline
I'll work on removing
import-error
As mentioned above I've opened PR #49013 which hopefully removes the need to disable import-error.
Comment From: slackline
I was trying to work out what had been taken and what was still available, knocked up this table and thought it might be useful.
| Pylint Error | Worked on by... | PR | Comment |
|---|---|---|---|
C |
|||
R |
|||
W |
|||
abstract-class-instantiated |
|||
access-member-before-definition |
|||
bad-super-call |
@soumilbaldota | #48896 | |
c-extension-no-member |
@roadswitcher | #49038 | |
function-redefined |
@FloHofstetter | #48906 | |
import-error |
@slackline | #49013 | Can't be removed see PR discussion. |
inherit-non-class |
@kostyafarber | #48897 | |
invalid-repr-returned |
@vamsi-verma-s | #49025 | |
invalid-unary-operand-type |
|||
misplaced-bare-raise |
@vamsi-verma-s | #49018 | |
no-member |
@TejasCreative | Can't be removed see comment | |
no-method-argument |
@gulyapulya | ||
no-name-in-module |
@roadswitcher | Can't be removed see comment | |
no-self-argument |
@Exoutia | #48912 | |
no-value-for-parameter |
@shalearkane | #48917 | |
non-iterator-returned |
@roadswitcher | #49036 | |
not-an-iterable |
@roadswitcher | #49031 | |
not-callable |
@joaomarcoscrs | ||
redundant-keyword-arg |
|||
too-many-function-args |
Not possible to turn off at present. | ||
undefined-variable |
@rAwsam | ||
unexpected-keyword-arg |
@shalearkane | ||
unpacking-non-sequence |
|||
unsubscriptable-object |
@sakalmon | Can't be removed see comment | |
unsupported-assignment-operation |
@jetale | #48982 | |
unsupported-membership-test |
|||
used-before-assignment |
@shivamurali / @Leviob | #48895 / #48940 |
Comment From: techrajdeep
@slackline , I am looking for an issue for my first contribution , is there anything available here. Can you please help me
Comment From: slackline
@techrajdeep rows in the above table are pylint errors that have not been worked on yet. See the original post in this issue for how to go about investigating the source of the errors. If you're not familiar with the concept of linting and/or pylint then it would be worth checking the documentation.
Comment From: vamsi-verma-s
worked on misplaced-bare-raise
Comment From: vamsi-verma-s
working on invalid-repr-returned
Comment From: roadswitcher
Working on not-an-iterable
Edit/Update: Found a solution that suppresses the error, but I think it just looks odd and is probably exposing a problem in pylint somehow. Deferring to @MarcoGorelli , closed associated PR, going back to trying to figure out how to start tacking W0621 errors.
Comment From: roadswitcher
Working on non-iterator-returned now
Comment From: roadswitcher
Checking c-extension-no-member now
Comment From: roadswitcher
Checking C now.
Comment From: roadswitcher
Checking
Cnow.
1323 separate files, >40k identified problems. I suggest that check is a candidate to leave disabled.
(pandas-dev) root@007e63d6aeef:/home/pandas# pre-commit run pylint --all-files | tee pylint-output
(pandas-dev) root@007e63d6aeef:/home/pandas# cat pylint_output | grep " Module" | wc -l
1323
(pandas-dev) root@007e63d6aeef:/home/pandas# cat pylint_output | wc -l
44157
I'm checking
RandWnow.
UPDATE: R has 589 files affected, W has 875.
Comment From: MarcoGorelli
thanks @roadswitcher for checking!
C, R, W are codes which have a lot of codes behind them. A PR to replace 'C' in pyproject.toml with all the individual codes would be very welcome (likewise for R and W)
Alternatively, disable= could be kept as it is, and a block with
enable = [
"W0621",
]
after it would be welcome
I'm very keen on enabling is W0621 (redefined-outer-name), as that one would've helped prevent a bug in pandas 1.5.0. Even just choosing 2-3 files at a time and fixing up all the W0621 errors from that one would be welcome - you can run just that error with
pylint pandas --disable=all --enable=redefined-outer-name
Comment From: jpfa1406
I'll work in redundant-keyword-arg
Comment From: ayushanand308
HI! I am a first time contributor. Can anyone assign me a beginner friendly issue so that I can work on it.
Comment From: MarcoGorelli
you can work on this issue without being assigned it. if you want to work on a different issue, you can comment there
Comment From: vamsi-verma-s
All abstract-class-instantiated errors are because of pandas.ExcelWriter .
https://github.com/pandas-dev/pandas/blob/e3d0126e8af9f0a23dd6609a23817f858825e089/pandas/io/excel/_base.py#L1090-L1110
This is recommended way to create writer objects. Should this be considered acceptable and disable this error?
To write a single object to an Excel .xlsx file it is only necessary to specify a target file name. To write to multiple sheets it is necessary to create an ExcelWriter object with a target file name, and specify a sheet in the file to write to.
Comment From: vamsi-verma-s
worked on access-member-before-definition
Comment From: roadswitcher
I took a look at enabling the invalid-unary-operator-type check, and I think it's exposed a problem where pylint doesn't properly recognize some NumPy constructs. Looking at the output, it is really curious as to why pylint's alerting on the same instance of the unary ~ multiple times on the same line.
************* Module pandas.core.strings.accessor
pandas/core/strings/accessor.py:594:37: E1130: bad operand type for unary ~: NoneType (invalid-unary-operand-type)
pandas/core/strings/accessor.py:594:37: E1130: bad operand type for unary ~: NoneType (invalid-unary-operand-type)
pandas/core/strings/accessor.py:594:37: E1130: bad operand type for unary ~: NoneType (invalid-unary-operand-type)
pandas/core/strings/accessor.py:594:37: E1130: bad operand type for unary ~: NoneType (invalid-unary-operand-type)
pandas/core/strings/accessor.py:594:37: E1130: bad operand type for unary ~: NoneType (invalid-unary-operand-type)
************* Module pandas.core.groupby.groupby
pandas/core/groupby/groupby.py:2055:32: E1130: bad operand type for unary ~: NoneType (invalid-unary-operand-type)
pandas/core/groupby/groupby.py:2055:32: E1130: bad operand type for unary ~: NoneType (invalid-unary-operand-type)
pandas/core/groupby/groupby.py:2055:32: E1130: bad operand type for unary ~: NoneType (invalid-unary-operand-type)
pandas/core/groupby/groupby.py:2055:32: E1130: bad operand type for unary ~: NoneType (invalid-unary-operand-type)
pandas/core/groupby/groupby.py:2055:32: E1130: bad operand type for unary ~: NoneType (invalid-unary-operand-type)
A search of the pylint issue tracker reveals they've encountered this behavior before ( and fixed previously ).
I think the check should remain disabled for now, but I'm working on a minimal test case to submit as an issue to the Pylint project.
Comment From: PedroPUCRIO
HI! I am a first time contributor. Can I check unpacking-non-sequence?
Comment From: Leviob
I'm working on expanding the "C", "R", and "W" codes into their respective errors. I'll keep each of these grouped together in pyproject.toml so it is more clear which lines are disabling errors vs conventions, refactors, or warnings.
Comment From: Leviob
After expanding the "C", "R", and "W" message types into their individual message names, here are the new pylint messages that can be worked on. https://github.com/pandas-dev/pandas/blob/d2acdee72b33c8c52bb5f8b902190bbfcf27ca47/pyproject.toml#L81-L204
I'm going to work on superfluous-parens next.
Comment From: Leviob
Pylint produces nearly 20k messages for invalid-name. Most of these are due to variables named with less than 3 characters, and not necessarily violating naming rules. Unless pylint is changed to accept short variable names, or we want to rename every df, we probably can't enable it.
See pylint issue: https://github.com/PyCQA/pylint/issues/2018
Comment From: fabinca
I'm working on the following R-Type pylint messages "consider-using-from-import", "consider-using-get", "consider-using-in",
Comment From: natmokval
Hi! I'm working on the “wrong-import-order” C-Type pylint message.
Comment From: stellalin7
I can work on the "disallowed-name" C-type pylint message
Comment From: manny-uncharted
take
Comment From: MarcoGorelli
no need to comment "take" as multiple people can work on this concurrently
Comment From: natmokval
PR for “wrong-import-order” is created: https://github.com/pandas-dev/pandas/pull/49390
Comment From: natmokval
I'll work on the "useless-return" message.
Comment From: natmokval
I'm going to work on the "nan-comparison" pylint message.
Comment From: natmokval
I'll work on the type "W" pylint warning: unnecessary-pass.
Comment From: natmokval
I'm going to work on the type "W" pylint warnings: wildcard-import, unused-wildcard-import.
Comment From: roadswitcher
Working on W warnings broad-except, and cell-var-from-loop
Comment From: natmokval
I'm working on the following R-Type pylint messages: literal-comparison, no-self-use, and invalid-sequence-index.
Comment From: natmokval
Hi! I’ll work on consider-using-sys-exit.
Comment From: natmokval
I'm going to work on the type "R" pylint warning: comparison-of-constants
Comment From: natmokval
I’ll work on the type "W" pylint warning: duplicate-value.
Comment From: MarcoGorelli
Note for anyone working on this: pylint is now run only in CI, so you'll need to run the command as
pre-commit run pylint --hook-stage manual --all-files
Comment From: natmokval
Note for anyone working on this:
pylintis now run only in CI, so you'll need to run the command as
pre-commit run pylint --hook-stages manual --all-files
Hi @MarcoGorelli . I noticed a typo in the above command. The following command works for me
pre-commit run pylint --hook-stage manual --all-files
Comment From: MarcoGorelli
you're right, thanks @natmokval !
Comment From: uzzell
I'll work on the type "W" warning import-self.
Comment From: fabinca
I'm working on --use-implicit-booleaness-not-len
Comment From: MarcoGorelli
Hi @fabinca - sorry, I'll add this one to the list of checks to keep turned off, after looking at it it doesn't seem to add any value https://stackoverflow.com/a/43476778/4451315
Comment From: MarcoGorelli
I've opened a separate issue for redefined-outer-name, in case anyone here wants to contribute towards that one: https://github.com/pandas-dev/pandas/issues/49656
It'll take a while to enable it, but I really think it'll be worth it
Comment From: natmokval
I'm working on self-assigning-variable
Comment From: uzzell
I looked at comparison-with-itself. If I counted correctly, there were 72 warnings, all of which were false positives. Does it make sense to keep it disabled?
Comment From: MarcoGorelli
Thanks @uzzell , yeah let's keep that one off thanks
Comment From: seanjedi
I'm working on wrong-import-order
Edit: Sorry, I misunderstood the question, I will look it over again and add a new comment if I find something that I can do.
Comment From: MarcoGorelli
thanks but we're not doing that one, check the end of the issue description
Comment From: uzzell
I'll work on use-sequence-for-iteration.
Comment From: Leviob
I'll work on undefined-loop-variable.
I'll start working on no-else-return within the pandas/io directory.
Comment From: MarcoGorelli
thanks @Leviob - some people aren't keen on no-else-* so let's leave that one out for now
Comment From: uzzell
I'll work on useless-parent-delegation.
Comment From: JasmandeepKaur
I'll work on line-too-long
Comment From: MarcoGorelli
hi @JasmandeepKaur - that one's already covered by flake8, no need
Comment From: JasmandeepKaur
Hello, Thanks for letting g me know! I'll look for another
Comment From: natmokval
I'm working on the following pylint messages: ungrouped-imports, invalid-envvar-default
Comment From: natmokval
If @joaomarcoscrs doesn't mind, I'll work on not-callable
Comment From: angularOcean
I wanted to work on unused-argument, unused-import, andunused-variable, but I got an error when trying to run pre-commit run pylint --all-files. I get the message "No hook with id 'pylint' in stage 'commit'", anyone else run into this issue? I forked the repo, made a branch and am running it on the branch as outline in the developer guide.
Comment From: MarcoGorelli
you need --hook-stage=manual too
let's not touch anything related with imports
Comment From: angularOcean
Thanks, I will leave unused-import alone. I tried pre-commit run pylint --hook-stages manual --all-files but now I get this error: "pre-commit: error: unrecognized arguments: --hook-stages manual"
Comment From: MarcoGorelli
sorry, --hook-stage
Comment From: angularOcean
Thanks --hook-stage fixed the issue. Also, after running a check on unused-argument, there turned up several hundred lines with the issue and from a quick look of a sampling of them, a large proportion of them are false positives i.e. 'col' in:
def time_iteritems_indexing(self):
for col in self.df3:
self.df3[col]
I can mark this as a false positive but there would be a lot of them. Would it be better to continue keeping this warning turned off?
Comment From: MarcoGorelli
we can probably keep it off
We're probably scraping the barrel here with this issue, I think we've probably enabled most of the most meaningful ones
There's still some files to do in https://github.com/pandas-dev/pandas/issues/49656 if you fancy that though
Comment From: seanjedi
Is anyone working on unsupported-membership-test?
Can I take that task?
Comment From: angularOcean
I took a look and I think use-implicit-booleaness-not-comparison is worth working on too
Comment From: angularOcean
I will also work on use-implicit-booleaness-not-len and see if that can enabled as well
Comment From: natmokval
Hi @MarcoGorelli , I checked pylint warning singleton-comparison and suggest keeping it off. Only false-positive warnings are raised on 8 lines (in 4 files); on every line, flake8 warnings, such as noqa: E711 or noqa: E712, are ignored.
For example: https://github.com/pandas-dev/pandas/blob/main/pandas/tests/series/test_arithmetic.py#L827
https://github.com/pandas-dev/pandas/blob/main/pandas/tests/frame/indexing/test_where.py#L828
Comment From: seanjedi
Hi @MarcoGorelli
I've started to work on unsupported-membership-test if that is alright.
I started working on it here: https://github.com/pandas-dev/pandas/pull/49848
I have not opened up the PR for review yet since there are still some issues I am trying to debug, once I have them resolved then I will open up the PR.
Comment From: MarcoGorelli
thanks @seanjedi - let's keep that one off too, I think they're all false positives and we risk annoying contributors
pointless-string-statement might be a good one - strings are sometimes used as comments, and it's probably worth converting them to comments because this check found an issue when I tried it
Comment From: CerqueiraMatheus
I will work on access-member-before-definition
Comment From: seanjedi
@MarcoGorelli Alright, thanks for letting me know!
I will work on pointless-string-statement then.
I will create a new branch, and if anyone wants to check what I have done for unsupported-membership-test then they can checkout my branch.
I agree though, that a lot of the changes were not really necessary, and would complicate some of the code.
Comment From: seanjedi
@MarcoGorelli I have worked on implementing the changes for pointless-string-statement here: https://github.com/pandas-dev/pandas/pull/49880
Comment From: roadswitcher
Found just over 30 instances of broad-except which appeared to be false positives -- do we want to keep this warning disabled?? (ref PR #49910 for example)
Comment From: uzzell
I'll take unnecessary-list-index-lookup.
Comment From: MarcoGorelli
Found just over 30 instances of
broad-exceptwhich appeared to be false positives -- do we want to keep this warning disabled?? (ref PR #49910 for example)
yeah let's do that
Comment From: roadswitcher
Found just over 30 instances of
broad-exceptwhich appeared to be false positives -- do we want to keep this warning disabled?? (ref PR #49910 for example)yeah let's do that
PR closed.
Comment From: bang128
Has anyone taken line-too-long yet? Can I work on it?
Comment From: MarcoGorelli
Thanks everyone for your contributions!
I think we've probably taken the most valuable ones already, so I think we can stop asking for contributions on this one.
If anyone comes across a pylint code which can be removed and which adds then value, then please do submit a PR to fix it! You're still welcome to do that, I just think it's time to close the issue
Thanks all 🙏