Skip to content

feat: adding more type annotations #3254

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

Merged
merged 15 commits into from
Apr 14, 2025
Merged

feat: adding more type annotations #3254

merged 15 commits into from
Apr 14, 2025

Conversation

gvwilson
Copy link
Contributor

@gvwilson gvwilson commented Apr 1, 2025

This PR will address #3226. Using pyright --pythonversion 3.9 ./dash to check.

@gvwilson gvwilson added P1 needed for current cycle fix fixes something broken labels Apr 1, 2025
@gvwilson gvwilson self-assigned this Apr 1, 2025
@gvwilson
Copy link
Contributor Author

gvwilson commented Apr 1, 2025

The warning in dash/table/__init__.py can be fixed by changing the imports at the top from:

from ._imports_ import *  # noqa: E402, F401, F403
from ._imports_ import __all__ as _components
from . import Format  # noqa: F401, E402
from . import FormatTemplate  # noqa: F401, E402

to:

from . import DataTable  # noqa: F401, E402
from . import Format  # noqa: F401, E402
from . import FormatTemplate  # noqa: F401, E402

However, this file is generated during npm run build, so we need to figure out where and how to modify the generator.

@gvwilson gvwilson force-pushed the pyright-typing-fixes branch from 45ca204 to 993283b Compare April 7, 2025 18:19
@gvwilson gvwilson marked this pull request as ready for review April 7, 2025 18:21
gvwilson added 2 commits April 8, 2025 09:24
Based on feedback from @T4rk1n, removed assertions added to satisfy
type checker and used other mechanisms instead.
@@ -1,5 +1,6 @@
import traceback
from contextvars import copy_context
from multiprocess import Process # type: ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

multiprocess is an extra dependencies, due to how we load everything in the dash.__init__ it would be imported in environment that hasn't installed it. Need to move back into the function call.

This library also has unparsable code so the typing/linting can't find the member and need this line back:

# pylint: disable-next=import-outside-toplevel,no-name-in-module,import-error

Copy link
Contributor

@T4rk1n T4rk1n left a comment

Choose a reason for hiding this comment

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

💃

@T4rk1n T4rk1n merged commit 245249a into dev Apr 14, 2025
3 checks passed
@T4rk1n T4rk1n deleted the pyright-typing-fixes branch April 14, 2025 12:43

import flask

from flask.typing import RouteCallable

Choose a reason for hiding this comment

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

This is not present in flask 1.1.1 and requirements are set to 1.0.4

Flask only has typing after 2.0.0

Choose a reason for hiding this comment

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

RouteCallable in flask.typing was added by PR pallets/flask#4676 which was released in flask 2.2.0 https://github.com/pallets/flask/commits/2.2.0?after=b17bb9ed563ab2857c0db9a07ec4e6407404c7be+34, so probably need to update requirements to >= 2.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix fixes something broken P1 needed for current cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants