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

20 - AttributeError: Can't pickle local object 'augment_visit.<locals>.augment_func' #21

Merged
merged 11 commits into from
Dec 28, 2021

Conversation

dineshtrivedi
Copy link
Contributor

Hello,

This is my attempt at fixing the AttributeError when trying to run pylint in parallel.
I have converted the closure methods/functions to be classes implementing call so they can now be pickled. I then wrote a unit test to assert that. If you put the old code back the test fails.

I also copied part of the pre-commit and tox.ini configuration from pylint project. I hope that is fine

I still need to do a bit more testing, but I thought of starting the PR and getting feedback on the changes. I would really appreciate any feedback.

Main Changes:

  • Replaced closure+function to keep state with classes implementing _call
  • Removed conditional support for python2
  • Removed unused code variable as it seems to be dead code and there are no unit tests indicating what that code entails

Project level and development environment changes:

  • Added pre-commit + Fixed all black and isort issues
  • Added initial version for setup.cfg
  • Added initial documentation for testing

…g __call__

- Added initial version for setup.cfg
- Added initial documentation for testing
@dineshtrivedi
Copy link
Contributor Author

relates to #20

.travis.yml Outdated
@@ -4,6 +4,7 @@ python:
- "3.5"
- "3.6"
- "3.7"
- "3.8"
Copy link
Collaborator

Choose a reason for hiding this comment

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

The setup.py lists supported versions as 3.6 to 3.10 (which is good!) so the CI config should match.

"""

if sys.version_info[0] <= 2:
checker = get_checker(linter, checker_method.im_class)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good that this old v2 compatibility code is going :)

@carlio
Copy link
Collaborator

carlio commented Dec 24, 2021

This all looks great :) Thank you @dineshtrivedi .

Seems like you're still adding changes so I'll wait till you're finished but as long as it keeps API functionality used by pylint-django (which as far as I can tell it does) then I'll merge and make a new release.

@dineshtrivedi
Copy link
Contributor Author

Thank you @carlio

I have made the change related to the ci. I have added the other supported python version and changed the coverage command to use pytest-cov

However, it seems the build is failing in Travis - https://travis-ci.org/github/PyCQA/pylint-plugin-utils/requests
Unfortunately, I can't see why. I wonder if it is because of this message:
image

It seems we need to move to www.travis-ci.com

Adding to that:

  • The coveralls badge is also not finding the project. Not sure what to do about this
  • Code health badge is also broken. Should I remove it?

Do you know how we can fix these issues?

Pending: Proper test with pylint-django

@carlio
Copy link
Collaborator

carlio commented Dec 24, 2021

Hmm, seems like travis refuses to build now without a paid plan for PyCQA which I'm not in charge of. I don't have the option to enable pylint-plugin-utils on travis right now sadly.

I think the best idea would be to move to using github actions for now. If you want to try that go ahead, otherwise I'll try to sort it out in the coming days.

For coveralls, I've enabled it in coveralls (turns out it wasn't enabled!) but it won't work until coverage is submitted, which won't work until CI is working.

For the health badge, might as well remove it. I still have vague plans to resurrect landscape.io but for now, just remove the badge and I can add it back if it every actually works!

@carlio
Copy link
Collaborator

carlio commented Dec 24, 2021

@dineshtrivedi I just made a dummy CI file in the master branch using the defaults. I'm not sure how github actions works, I figured using the example might be enough to get it started. If you rebase/merge master into your branch and overwrite the ci.yml with the changes you made, that should kick things off I guess.

@carlio
Copy link
Collaborator

carlio commented Dec 24, 2021

Wait ignore me I am stupid :-) your CI is in your repo fork of course!

@dineshtrivedi
Copy link
Contributor Author

dineshtrivedi commented Dec 24, 2021

I have copied part of the ci.yml from pylint project. I also don't have experience with github actions.
It seems to be working on my side - https://github.com/dineshtrivedi/pylint-plugin-utils/actions

And the changes seem to not have broken pylint-django when I tested.

I think for now I am done @carlio
Any other changes I can make in another PR with fewer changes so it is easier to review and test.

Let me know if there is anything else I can assist with

@carlio
Copy link
Collaborator

carlio commented Dec 24, 2021

Thanks again @dineshtrivedi . I will get back to this in a couple of days after Christmas but I don't expect any problems.

@dineshtrivedi
Copy link
Contributor Author

Sure, @carlio. Thank you for that
Have a good festive season :)

@carlio carlio merged commit 2f26141 into pylint-dev:master Dec 28, 2021
netbsd-srcmastr referenced this pull request in NetBSD/pkgsrc Apr 24, 2022
Version 2.5.3 (25 Mar 2022)
---------------------------
Bugfixes
- Fixed compatibility issue between pylint `2.12` and `2.13` to construct `ScopeConsumer` tuples correctly depending on version

Other
- pylint version is now pinned to `<3` to give breathing space to update pylint-django before the major update lands


Version 2.5.2 (18 Feb 2022)

Bugfixes
- Fixed crash with assigning a class to a variable, and referencing the variable when subclassing

Other
- CI now tests against Django 4.0


Version 2.5.1 (16 Feb 2022)
---------------------------
Bugfixes
- Fixed pickling for `augment_visit`


Version 2.5.0 (02 Jan 2022)
---------------------------
Bugfixes
- Fixed compatibility with astroid 2.9.1

New
- Removed false positive error of missing member in TextChoices tuples
- Moved from Travis CI to GitHub Actions
- Added pre-commit configuration and began enforcing black/isort code formatting
- Multiple test fixes
- Bumped dependency for pylint-django-utils to get `multi-threaded pylint support <https://github.com/PyCQA/pylint-plugin-utils/pull/21>`_
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.

2 participants