-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Deduplicate module file paths to prevent redundant scans. #7747
Conversation
Closes pylint-dev#6242 May also address pylint-dev#4053
Pull Request Test Coverage Report for Build 3495785504
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for contributing, much appreciated !
With 'path' as the key, we get deduplication for free and do not need to reprocess the list for deduplication later.
…, document how the handling works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you check if it also fix #689, please ? Maybe we also need to sort the file to be stable between interpreters / operating system. Maybe a dict with an interpreter > 3.7 is enough.
This comment has been minimized.
This comment has been minimized.
Do we want to sort the files? As a user of Pylint, I would prefer that it respects the order in which I specify arguments. Anyway, here's the output I get if I use the reproducer provided earlier this year:
So, the outputs are identical except for ordering. This looks right to me. And, if I disable all of the checks that the reporter did not show, then I see:
I.e., identical output in spite of ordering. I do not know if this patch can take credit for the fix, but the issue does seem to be fixed one way or another. |
I just checked against the other reproducer in #689 - there is still a problem with which check appears to be associated with which module, but there are no extra check message depending on order:
So, I think that #689 or some variant thereof must remain open. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you. I have a suggestion to appease mypy, let me know what you think :)
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great first contribution, on a nasty bug that was open for far too long 👍 (Probably a pre-requisite to fix the multiprocessing too, another very important issue). Final nitpick and let's merge, maybe even in 2.15.6 :)
This comment has been minimized.
This comment has been minimized.
🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉 This comment was generated for commit c67f204 |
…#7747) * Use dict for 'expand_modules' result rather than list. With 'path' as the key, we get deduplication for free and do not need to reprocess the list for deduplication later. * Fix deduplication to account for CLI args marker. * Fix corner case with CLI arg flag handling during deduplication. * Add 'deduplication' to custom Pyenchant dict. Closes pylint-dev#6242 Closes pylint-dev#4053 Co-authored-by: Eric McDonald <[email protected]> Co-authored-by: Pierre Sassoulas <[email protected]>
* Use dict for 'expand_modules' result rather than list. With 'path' as the key, we get deduplication for free and do not need to reprocess the list for deduplication later. * Fix deduplication to account for CLI args marker. * Fix corner case with CLI arg flag handling during deduplication. * Add 'deduplication' to custom Pyenchant dict. Closes #6242 Closes #4053 Co-authored-by: Eric McDonald <[email protected]> Co-authored-by: Pierre Sassoulas <[email protected]>
Maintain and reference a set of visited files while expanding the module files from list of module directories, package names, and standalone files to prevent duplicate scans.
Type of Changes
| ✓ | 🐛 Bug fix |
Description
Closes #6242, #4053