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

Incompatible with mypy>=0.730 #345

Closed
ziyadedher opened this issue Sep 30, 2019 · 12 comments
Closed

Incompatible with mypy>=0.730 #345

ziyadedher opened this issue Sep 30, 2019 · 12 comments

Comments

@ziyadedher
Copy link

ziyadedher commented Sep 30, 2019

Describe the bug
Prospector is incompatible with mypy versions 0.730 and above. mypy==0.730 introduces an "error summary" statistic in the output that is enabled by default (see this commit). This breaks the mypy module message parsing here due to trying to unpack too many values.

To Reproduce
Steps to reproduce the behavior:

  1. pip install 'prospector[with_everything]'
  2. pip install mypy==0.730 (should already be installed)
  3. prospector

Expected behavior
Prospector should either be compatible with receiving this line from mypy or should pass in the argument --no-error-summary by default. A naive --no-error-summary approach might break compatibility with mypy<0.730, but it would go here.

Environment (please complete the following information):

  • OS: Arch Linux
  • Tool mypy
  • Prospector version 1.1.7
  • Python version 3.7.4

Additional Information
Currently, the best workaround I found is to just downgrade mypy to 0.720.

@ziyadedher
Copy link
Author

ziyadedher commented Sep 30, 2019

Drafted a simple pull request to address this situation. Feel free to modify it. #346

rik added a commit to rik/prospector that referenced this issue Oct 10, 2019
Mypy now returns a summary as the last line of the output. This patch filters them out.

fix prospector-dev#345
@rik
Copy link
Contributor

rik commented Oct 10, 2019

I've submitted #349 for an alternate approach that I think is safer.

@ziyadedher
Copy link
Author

I've submitted #349 for an alternate approach that I think is safer.

I think this is a better approach than mine, but it might introduce incompatibilities if Mypy ever changes their summary line message.

@rik
Copy link
Contributor

rik commented Oct 11, 2019

If it happens, we can always change that logic.

@chocoelho
Copy link
Contributor

As I'm leaning towards the latter PR, I suggest we pin mypy as it updates constantly and keeping tracking of these changes can be recurrent and will tend to promptly break CIs across projects. We can pin using <=0.730 for now. How do you feel @rik and @ziyadedher ?

@chocoelho
Copy link
Contributor

Whenever we can set on this I can work on releasing 1.1.9 instead of 1.2.0 for now.

@rik
Copy link
Contributor

rik commented Oct 15, 2019

I'm not very fond of meta-tooling putting restrictions on what version underlying tools can use.

Pros Cons
Pinning Guarantee prospector will always work Downstream can't use newer version until new Prospector release
Not-pinning New compatible versions are immediately available Prospector can break with new release. Downstream can pin when it happens

I feel a tool like prospector should strive to keep up with latest versions otherwise it will get in the way of projects using it. As an example, I think I've ran into pylint bugs that are resolved in pylint 2.4+ but prospector pins 2.3.1.

Whenever a tool is incompatible with prospector, maybe temporarily updating the README file with workarounds is an acceptable trade-off?

I say all of that as a user, not a maintainer, so maybe the burden of not pinning is too much.

@chocoelho
Copy link
Contributor

@rik as you may have noticed, the burden can increase a lot by not pinning. As it's usually a few people making the updates, this can be more than we can take as of this moment. I'm working on upgrading to the latest pylint version, alongside the other tools, instead of adding tools/functionalities in the next couple days so we can see how unpinning goes from there.

@rik
Copy link
Contributor

rik commented Nov 12, 2019

My pylint example was poorly worded. I wanted to illustrate that by pinning versions, users of prospector don't see the issues with newer versions. By not pinning, users run into the issues and may contribute patches to help keep up. I know that was a big factor for my few contributions here.

Regardless of the pinning discussion, would it be possible to merge one of the two PRs for this issue and release a new version of prospector?

@chocoelho
Copy link
Contributor

chocoelho commented Nov 18, 2019

@rik I totally get you. This may be the way, but I think we may loose a bit the restriction one dep at a time. We can start off with mypy. Can you try updating prospector to 1.2.0.dev1 1.2.0.dev2?

@rik rik mentioned this issue Nov 18, 2019
rik added a commit to rik/prospector that referenced this issue Nov 18, 2019
Mypy now adds a summary as the last line of the output. Pass a flag to not add this symmary.

Previous versions of mypy ignore this flag.

fix prospector-dev#345
@rik
Copy link
Contributor

rik commented Nov 18, 2019

I'm really sorry, I made the silly mistake of doing a little "code cleanup" in my PR and it turned out to not work (It doesn't matter how many times I learn this lesson, I keep falling in this trap). So 1.2.0.dev2 breaks all mypy versions.

I've submitted #355 to revert the change and use the flag approach.

@chocoelho
Copy link
Contributor

No worries, that's why I've started doing dev releases instead of just releasing things. I've done the same thing before 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants