-
-
Notifications
You must be signed in to change notification settings - Fork 525
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
Release 9.0 breaks mypy (and pylint etc.) #940
Comments
I expect this is caused by the new lazy import system. Another consequence of this is that trying to jump to definitions no longer works (one always lands in the package dunder init file) which is unfortunate since the ability to quickly inspect third party code is one of my favourite features of python. |
Oh, the joy. |
The lazy importer also breaks pylance, similar to the error for mypy, as well as pyinstaller as it fails to include the correct modules in the distribution. as a work around I'm currently using from websockets.legacy.client import connect as ws_connect which resolved both issues. |
This is the pragmatic workaround at this point. I hate that it exposes the https://stackoverflow.com/questions/60739889/bypass-mypys-module-has-no-attribute-on-dynamic-attribute-setting has a good discussion of the options and they're all bad :-( |
There are several options. Option 1 - do nothing "If you want to use mypy, use the full import paths, not the convenience imports from the root module." If it weren't for the temporary Option 1bis - hide Like option 1, but with a few imports added, like Type checking would still fail for deprecated APIs, though: I can't bypass the lazy import system because it's also taking care of raising deprecation warnings. I think this is my favorite option. Option 2 - provide type stubs The primary driver for the lazy import system is easier refactoring. Having to maintain type stubs would hinder that goal. |
Could you report whether a7b2400 works? Supported uses should include: import websockets.client
await websockects.client.connect(uri) and from websockets.client import connect
await connect(uri) (This will have to be a bugfix release on top on 9.0.1, but I never did maintenance branches in websockets, so this isn't a request.) |
You could have at least put a changelog file to say that the api was changing. Also semver would be nice. |
In any case, changing the import seems to work but doesn't satisfy mypy.
|
Didn't it change between 8.x and 9.x? That sounds like semver to me. Anyways, it seems to me this was not an api change but just a change to the way imports work. The side effect of making type checking break was obviously unintentional. |
Well imports are code. Specifically now I have the problem that distributions use one version, pip installs another and I don't know if I can make code compatible with both. Also mypy fails even after fixing the imports so that doesn't help so much.
If you see the releases, the version number changes almost all the time, meaning that either it's not semver or it breaks API all the time. |
@ltworf -
I did. That didn't help since you didn't read the changelog before complaining.
websockets follows semver. While semver doesn't work, some folks like it and and would complain otherwise; making them happy doesn't cost much.
Well that's just one of the reasons why semver doesn't work. Due to Hyrum's law, given enough users, many kinds of changes could be breaking change for some users, so the major version number changes often. I suspect some of these "possibly breaking" changes didn't actually affect anyone. Please consider the effect that the patronizing tone of your comments has on my willingness to work for you for free. The immediate effect is that my motivation to work on this bug, which doesn't affect me personally, dropped a lot 🤷 Thanks for pointing out that, even with the best intentions, mistakes can happen. |
I downloaded the .tar.gz and there is no changelog file, so unless you mean I clone the repo and check all the commits, there is no changelog.
I use your library in a open source project that I maintain in my free time and for which I have never received a single donation. So now because of your breaking changes I'm on a sunday debugging things because those changes completely break my project for my (non paying) users. |
@ltworf If this is really such a big issue for you then you should pin all of your dependencies. I too was affected by this and the solution was quite simple. Quickly make a commit to pin the dependency to I too maintain an open source project and it baffles me that you do as well but then choose to come harass the maintainer here. Since any change in a dependency could break end users, you should probably pin all of your dependencies or maintain forks of all of your dependencies so that you can properly vet all changes if a change like this is such a disruptive change for you. |
I agree that this is a good option and it appears to work for a simple example. However, if IDEs and other development tools prefer this pattern, then it could become widespread making the deprecation warnings less useful. Edit: removed hasty suggestion Edit: Another idea is to bypass lazy import system for type checking. It's cumbersome and kind of defeats the purpose of the lazy import, but it appears to work for mypy, pylint and pycharm alike: from typing import TYPE_CHECKING
if TYPE_CHECKING:
from websockets.legacy.client import connect
from websockets.legacy.client import WebSocketClientProtocol
# etc |
pinning dependencies and embedding stuff is a sure way of making sure your project will never be included in a distribution. I have done that but I also need to make a minor release so that potential users downloading the latest release get something that works. But long term this is no solution.
Stating that there is no changelog and that changing the import doesn't make mypy happy is facts, not harassment. It is providing relevant facts for the resolution of the issue. And again pinning dependencies is not a good thing and when developing a library it is good to keep compatibility in mind, when possible. I'm not saying all projects should be like the linux kernel and keep bugs for the sake of remaining compatible forever, but there is a middle ground between that and the opposite extreme of renaming all the functions every release. |
The conversation diverged quite a bit from the original issue so I'm going to lock it and go back to solving the issue. |
Thanks for testing and providing feedback! Also, yes, I briefly considered the
|
I just released 9.0.2 with the fix. |
Thanks a bunch.
|
Apologies for the disruption! |
For whoever ends up here: try this: from websockets.client import connect
from websockets.server import serve if mypy or other tools that don't understand the dynamic nature of Python give you false positives. |
For those using pylint, I wrote a(n experimental) pylint plugin for this |
Some more details can be found at python-websockets/websockets#940
Some more details can be found at python-websockets/websockets#940
@aaugustin I'm on v10.1 and this doesn't work with MyPy nor Pylance: https://i.imgur.com/y1UDAPl.png The only modules that work are Using Imports are persistent, meaning that once you import something once, any subsequent import is going to just link the already imported module from memory instead of executing it again. Alternatively, you can avoid import cycles by using just
I'm extensively using typed code and MyPy for early error detection and type hinting purposes, and the way it is right now, makes it literally impossible, short of modifying |
@DevilXD There's a typo in that screenshot. Try |
For the record, the lazy import mechanism is intended to avoid loading the server code when you're only going to use the client code and vice-versa. I am ready to admit that it's a nano-optimization no one cares about. The CPU and memory impact of loading an unused module isn't very high. That said, unless you're a Tidelift subscriber, you paid $0 for this software. For that price, you can probably bear with my quirks 🤷 I appreciate that you wanted to give me some context about how imports work in Python. I happen to have a bit of domain expertise in this area, especially import cycles. Lazy imports don't really help with import cycles. If anything, they make the situation more confusing, as they make the import sequence less deterministic. |
To be fair, you are quite free to release this as proprietary software. What I do is that I use GPL license, and I'm open to give LGPL exceptions for money (which has never once happened). In this way I know that people who don't pay are doing libre software, so they are welcome to use my stuff for free. Those who want to make proprietary stuff (and are most likely making money from it) generally end up not using my project. |
In fact I'm happy if people use websockets and make money with it (preferably in an environmentally responsible way). I'm not really trying to monetize it. When I bring up the price, it's usually a signal that the discussion crossed a frontier and I don't think the feedback is going to help me improve websockets. |
Hmm, that appears to have worked. I'm not sure how I've missed that yesterday, I guess it's my mistake then.
I see. I'm sorry if this came out harsh to you, but out of frustration, I seriously considered looking for a different package yesterday, one that would allow we to type check my code properly. If you really are happy that others use websockets, then it only makes sense to not make it unnecessarily difficult to use them. Am I right? |
Yes, it makes sense :-) I'm glad you're sticking with websockets :-) |
Some more details can be found at python-websockets/websockets#940
439dafa should solve the problem fully going forwards. |
Running a project that uses websockets 9.0 against mypy, throws:
for
and
for
Version 8.1 of websockets has no such issue. Using the latest mypy 0.812.
The text was updated successfully, but these errors were encountered: