Skip to content
This repository was archived by the owner on Jun 3, 2024. It is now read-only.

Plotly.js 1.55.1 #858

Merged
merged 6 commits into from
Sep 3, 2020
Merged

Plotly.js 1.55.1 #858

merged 6 commits into from
Sep 3, 2020

Conversation

Marc-Andre-Rivet
Copy link
Contributor

No description provided.

@nicolaskruchten
Copy link
Contributor

1.55.1 just came out

@Marc-Andre-Rivet Marc-Andre-Rivet changed the title Plotly.js 1.55.0 Plotly.js 1.55.1 Sep 3, 2020
@Marc-Andre-Rivet Marc-Andre-Rivet marked this pull request as ready for review September 3, 2020 01:32
@@ -70,7 +70,7 @@ def display_page(pathname):
time.sleep(2)

# callback is called twice when defined
self.assertEqual(call_count.value, 2)
self.assertEqual(call_count.value, 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happened here? Is this related to plotly/dash#1385 ? We should fix the comments on these two changed lines once this is understood. Any chance this means plotly/dash#883 or plotly/dash#1049 has incidentally been resolved?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexcjohnson Yes. This is caused by plotly/dash#1385.

  1. The initial callback gets added to the list of requested callbacks
  2. Before await wait(0); resolves the dcc.Location component renders and triggers a setProps because its props are not populated / misaligned with the current url (before, the callback would have been executing before the component rendered)
  3. An additional callback is added to the list of requested callbacks
  4. await wait(0); resolves
  5. There are now two callbacks for test-url.pathname and the oldest one gets pruned

Choose a reason for hiding this comment

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

I'm still seeing plotly/dash#1049 running what I think is the latest version of everything. Is it supposed to be resolved as of this merge?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The boundaries of the problem have changed a little, as evidenced by this test, but it’s definitely not fixed.

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

💃 once the test changes (unrelated to the plotly.js upgrade) are understood and the comments fixed.

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

Successfully merging this pull request may close these issues.

4 participants