-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Version 3 adds breaking type annotations that do not comply with mypy #3226
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
Comments
I only had trouble with mypy, the typing generation tests were originally designed with mypy but at some point it updated and couldn't find types anymore. There is a I suggest switching over to |
I'll give that a look but we can not simply dump mypy from our code base. E.g. looking at https://github.com/plotly/dash/blob/dev/dash/dependencies.py#L29 and following it is clear, that there are no type annotations for |
There are several issues with the claimed "typedness" for dash. In a fresh virtual environment I get: $ mypy .
dash-core-components is not a valid Python package name
$ cd dash
mypy: "types.py" shadows library module "types"
note: A user-defined top-level module with name "types" is not supported
$ cd ..
$ pyright
# see https://gist.github.com/gothicVI/34eb60a2a2f61b02d6a2c50027355a11 for very long output
$ cd dash
$ pyright
# see https://gist.github.com/gothicVI/b2f30b6cbfbc648909eb3f82c4560c2c for lengthy output which tells me that the packages violates python conventions and is not typed. |
Hi @gothicVI - thanks for filing this issue. We're aware that there's still work to do on Dash typing, and given resources at our end we have to take it in stages. If you would like to post a PR with a handful of the changes you would like to see, we can review that right away, and it will help us figure out how much effort it will be to solve the whole problem. Thanks - @gvwilson |
Hi @gvwilson thanks for taking a look. To be completely honest I do not know how to start contributing in that regard because I do not fully understand the internal workings yet. I might give it a look but can't prommis any PR. I think the most needed changes would be to figure out what exactly breaks mypy when changing to version 3. Don't get me wrong, mypy did complain earlier due to there not being any annotations or stub files so we commented that out as mentioned above. But the release of version 3 did break that which it shouldn't. If needed revert the changes for the moment! After that I'd suggest sticking to the general python conventions like no Please do not get me wrong. I love this project and this should not be understood as a personal attack against anyone who contributed typing code! Yet I think it is quite dangerous to push a major version upgrade without thorough testing - especially in such a large project that so many other projects rely on. |
HI @gothicVI - thanks very much for your quick feedback. Reverting these changes isn't an option - there are a great many features that our users have wanted for a long time - so I'd like to focus on ways to move forward. If you can start a PR to convert from |
@gvwilson I didn't mean to revert the release of version 3 but just the commits that break the typing system. Either way, can you or anyone else confirm that the new version breaks mypy at all? Independently I've looked into converting the My attempt at conversion can be found here: gothicVI@ed59db7 |
@gvwilson any news? |
Nothing yet, but I'll be sure to post when there is. |
@gothicVI can you please tell us more about the build issue you're having? when I go through the steps in |
@gvwilson I honestly can no longer reproduce the issue I had but it would fail due to I'll try again with Irrespectively, can you reproduce the initial issue though? |
Investigated the type checkers, I found that mypy doesn't even resolve the html/dcc types that were added while it works fine with pyright/vscode/pycharm. This error results from that:
It happens that Adding type vars can resolve the return type, but break the init types. Can probably refactor the decorator behavior inside the
Has been fixed in 3.0.2 by community PR: #3249
The dash dependencies will be typed in next dash release by #3259
This can be fixed/silenced with |
@T4rk1n that looks great. Any change to add |
I'll probably add a basic test for mypy to not throw error with a basic app and make sure it resolve the types of the components. But for the rest of the codebase it should run #3259 Also fix the component types, that may reveal actual typing errors in layout when using mypy. |
Some things seem to be fixed but other not so much:
|
You need to build the components and run it from where it can find dash if running locally. I get no more error when running this example. |
@T4rk1n damn good point. I have 0 experience with JS/TS though and am somewhat stuck with
while
|
@gothicVI what OS are you on? |
@gvwilson Manjaro Linux |
OK, I don't understand why that wouldn't work. Can you please check $ npx lerna clean |
@gvwilson here you go https://gist.github.com/gothicVI/8a8cb53ba0be2f060d30ebf004edbaeb I'll try and give it a shot with a machine running python |
@gvwilson might have fixed it with a small diff:
This allowed me to build the dependencies. For the full output see https://gist.github.com/gothicVI/5410fa76c5b177b048f8809a28b4773c Finally, leaving the cloned dash directory to check
|
thanks @gothicVI - as noted earlier, we're focusing on pyright for type checking. If there are changes you can PR that will satisfy mypy without breaking pyright, I'd be happy to prioritize review. |
hi @gvwilson the initial issue is not solved as you can see. All I solved was building dash using python |
@gvwilson you had mentioned that with your patch you didn't see the issues. What exactly did you do? |
With the release of dash 3.0 our CI/CD fails for stuff that used to work.
Here's a minimum working example:
running mypy on this file results in:
which we used to solve by adding
# type: ignore[misc]
to everycallback
call and by addingto our
pyproject.toml
.However, when updating to version 3, we get:
without changing anything else.
This can't be intended behavior, right? How to fix that?
community post: https://community.plotly.com/t/dash-3-0-fails-mypy/91308
The text was updated successfully, but these errors were encountered: