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

pin to astroid 2.1.0 since 2.2.0 can break users #2774

Closed
wants to merge 2 commits into from

Conversation

jab
Copy link
Contributor

@jab jab commented Feb 27, 2019

@PCManticore
Copy link
Contributor

Thanks, but as mentioned in the linked issue, this release is supposed to be using 2.2+.

@PCManticore PCManticore closed this Mar 2, 2019
@jab
Copy link
Contributor Author

jab commented Mar 2, 2019

Can you please confirm whether pylint adheres to semver? If so, my understanding is that if a project upgrades a dependency to a version that can cause a breaking change, the project should bump its major version to indicate this.

In any case, what are users who were broken by this supposed to do to make sure they stay on a compatible version of transitive deps pulled in via pylint? I'm manually pinning to astroid 2.1.0 to work around this but if semver is adhered to it shouldn't be necessary to specify any transitive deps.

@schrodervictor
Copy link

astroid 2.2.0 is also breaking everything in our setup. Manually pinning it to astroid 2.1.0 fixed the problem, but it's very annoying to have broken dependencies in pylint. I would strongly suggest to re-open the issue, as it seems to be affecting several users.

@PCManticore
Copy link
Contributor

@jab I just released pylint 2.2.2 which restricts the version of astroid that it needs, so the crash you noticed should be gone once it installs an older version of astroid. The current 2.3 and future 2.4 releases need astroid in order to work correctly across the board, not just because that method is gone, but there are vital perfomance issues in the last release of astroid that solves various problems that users had with large libraries such as pandas and numpy. Restricting the astroid version to 2.1 means that they would have to wait again for using a more recent astroid version.

Regarding semver and pylint, this is muddy waters for linters. Additional checks land all the time in minor releases. Otherwise we'd have to bump to a major release every time we want to add a new check, which seems like an unmaintainable approach to me. Apart of that, inference improvements land all the time in minor releases of astroid, which in turn can mean that:

  • checks that weren't emitted in previous minor versions of pylint can now be emitted in following minor version
  • previous false positives can be gone in a new release
  • or new false positives can appear due to the inference engine understanding more of Python that it did a former version.
    This is why we're not strictly following semver, otherwise all these changes would need to happen in major versions. As a rule of thumb, it's always the last version of pylint with the last version of astroid for getting the most out of pylint (or 1.9 which is still maintained plus the corresponding latest version of astroid that's "<2.0")

@bj00rn
Copy link

bj00rn commented Mar 5, 2019

@jab I just released pylint 2.2.2 which restricts the version of astroid that it needs, so the crash you noticed should be gone once it installs an older version of astroid.

@PCManticore awesome. But you mean 2.2.3 right?

@PCManticore
Copy link
Contributor

@bj00rn Yes, you are right, that's 2.2.3

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.

4 participants