-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Bumping d3 #424
Comments
@etpinard these are probably well known to all of us, so it's a 'just in case':
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Observations from an aborted attempt:
Beyond that, it's not like the logic is all that different. It's a lot of bouncing from one error to the next and translating the code. The result that I got was able to display a grid with dragging and some hoverlabels, so it's viable, just would take more work than seems appropriate in the short term. |
@rreusser nice summary! Yes, lots of relatively superficial upgrade work even with smaller codebases. Various incompatible changes, e.g. the collapsing of the namespaces, various renamings or both (e.g. Besides the I wonder if the Regarding the promise of smaller components, maybe this new tool can help customize what's included but I haven't yet tried. The reason it might work is that there are numerous modules not used in the codebase. In any case it's an almost negligibly small fraction of the Based on the current way we use D3 I wouldn't expect significant user-visible improvements from a switch. It can be useful for other reasons, I suppose the community (programmers, bug fixes) move to 4.0 over time. Or if there is a big modularization attempt in All in all |
Haha, but array access is soo easy! Come to think of it, I'm not sure it's actually used in the code. That might just be what I use to debug in the inspector. |
Yeah, the other change that maybe had consequences was transitions. The current animation PR falls back to instantaneous data updates when you transition the scale and the data at the same time. The reason seems to be a race condition in which the running transitions (points moving around) don't play well with transform changes (the whole layer moving around). It's difficult to debug because of the way v3 makes it relatively difficult to group transitions (i.e. It's ripe for debugging and refactoring, but the current priority is consistency and correctness, then handling more complicated interactions and corner cases. I think v3 will get us there. 👍 |
Good point. Transitions can be a huge thing to upgrade properly (the new is so much better, taking advantage of it may need full rethinking of the transitions). Since you wrote the animations you are the one who knows about the expected |
Referencing @rreusser 's #946 (comment) about our the very common |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
FYI d3 v5 should make its official debut soon: https://github.com/d3/d3/releases/tag/v5.0.0 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Is this topic up2date? - I would propose the following: instead of replacing all the d3-usages in the code I would create a intermediate-d3-wrapper which forwards functions used in plotly to the related functions in d3. In the wrapper itself we could make a version-check of d3 and depending on the version use the right d3-function. Also with this plotly could maybe support different versions and an upgrade to later versions of d3 are maybe easier. Does this idea/approach make sense somehow? |
This comment has been minimized.
This comment has been minimized.
Pasting #3052 (comment) For completeness, the items d3v3 is missing that v5 has are:
Perhaps not worth it until we upgrade our d3, but it would be possible for us to add |
d3-format v4+ has new formatting features ... eg.
|
if there's no real plans to update to the newer version of d3, is there some recommendation for users to still end up with a ISO8601 weeknum (%V)? |
Is there any plan to implement this? |
Also especially usefully in the new d3-format:
Finally, In order to have nice tick positioning and formatting while zooming, I had to use newer d3 (5.15) on the side. I imagine Plotly would just take care of it, once upgraded to a newer version of d3. Meanwhile, here is my code. // use d3 to compute ticks (a newer version than the one used by Plotly)
const xNiceScale = d3.scaleLinear().domain(xaxisRange).nice();
const xTickValues = xNiceScale.ticks();
const xTickFormatter = xNiceScale.tickFormat(xTickValues.length);
const xTickLabels = xTickValues.map(xTickFormatter);
const yNiceScale = d3.scaleLinear().domain(yaxisRange).nice();
const yTickValues = yNiceScale.ticks();
const yTickFormatter = yNiceScale.tickFormat(yTickValues.length);
const yTickLabels = yTickValues.map(yTickFormatter);
const layout = {
"xaxis.tickmode": "array",
"xaxis.tickvals": xTickValues,
"xaxis.ticktext": xTickLabels,
"yaxis.tickmode": "array",
"yaxis.tickvals": yTickValues,
"yaxis.ticktext": yTickLabels
}; |
This one is addressed in #5026 and would be available in |
A table of
|
🎉 and now d3-v6 is out: |
This issue has been tagged with A community PR for this feature would certainly be welcome, but our experience is deeper features like this are difficult to complete without the Plotly maintainers leading the effort. Sponsorship range: $70k-$80k What Sponsorship includes:
Please include the link to this issue when contacting us to discuss. |
Just to be clear... Plotly is based on a version (~3.x) of d3 from 2016... The current version of d3 is now 7.4.5. This seems like a major concern given that it's the foundational package for graphing. |
@emilykl before you move on, could you please create a confluence page with your findings? |
A discussion on what modules we need in d3 v4.0.0.
The text was updated successfully, but these errors were encountered: