Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix some typing issues #2841

Merged
merged 6 commits into from
Sep 10, 2024
Merged

fix some typing issues #2841

merged 6 commits into from
Sep 10, 2024

Conversation

DanielIskandar
Copy link
Contributor

Start with a description of this PR. Then edit the list below to the items that make sense for your PR scope, and check off the boxes as you go!

Contributor Checklist

  • I have broken down my PR scope into the following TODO tasks
    • task 1
    • task 2
  • I have run the tests locally and they passed. (refer to testing section in contributing)
  • I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR

optionals

  • I have added entry in the CHANGELOG.md
  • If this PR needs a follow-up in dash docs, community thread, I have mentioned the relevant URLS as follows
    • this GitHub #PR number updates the dash docs
    • here is the show and tell thread in Plotly Dash community

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove empty.

package.json Outdated
@@ -49,7 +49,7 @@
"devDependencies": {
"@lerna/filter-options": "^6.4.1",
"husky": "8.0.3",
"lerna": "^6.6.2"
"lerna": "^6.4.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to rollback to 6.6.2 > 6.4.1

Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is a public module, we cannot rename without it being a breaking change.

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.

This look good overall and a good addition, just need to address my previous comments and add a changelog entry.

Comment on lines +408 to +413
long_callback_manager: Optional[
Any
] = None, # Type should be specified if possible
background_callback_manager: Optional[
Any
] = None, # Type should be specified if possible
Copy link
Contributor

Choose a reason for hiding this comment

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

Both of those should be of type dash.long_callback.managers.BaseLongCallbackManager

Comment on lines +397 to +400
meta_tags: Optional[List[Dict[str, Any]]] = None,
index_string: str = _default_index,
external_scripts: Optional[List[Union[str, Dict[str, Any]]]] = None,
external_stylesheets: Optional[List[Union[str, Dict[str, Any]]]] = None,
Copy link
Contributor

Choose a reason for hiding this comment

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

Those Any in the Dict would be str

@@ -375,40 +378,43 @@ class Dash:
STARTUP_ROUTES: list = []

Copy link
Contributor

Choose a reason for hiding this comment

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

we can add server: flask.Flask here.

@gvwilson gvwilson mentioned this pull request May 29, 2024
@gvwilson gvwilson changed the title Typesetting issues typing issues Jun 6, 2024
@gvwilson gvwilson changed the title typing issues fix some typing issues Aug 13, 2024
@gvwilson gvwilson added P2 considered for next cycle fix fixes something broken community community contribution labels Aug 13, 2024
@gvwilson
Copy link
Contributor

@DanielIskandar are you interested in updating this one so that we can merge it? thanks - @gvwilson

@T4rk1n T4rk1n merged commit 9323c28 into plotly:dev Sep 10, 2024
3 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
community community contribution fix fixes something broken P2 considered for next cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants