Skip to content
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

Event data for transformed traces is inconsistent #1807

Closed
rreusser opened this issue Jun 21, 2017 · 13 comments
Closed

Event data for transformed traces is inconsistent #1807

rreusser opened this issue Jun 21, 2017 · 13 comments
Labels
bug something broken

Comments

@rreusser
Copy link
Contributor

rreusser commented Jun 21, 2017

This plot is throws an error whenever I hover in 1.28.2:

https://codepen.io/rsreusser/pen/bRRBjg

Uncaught TypeError: Cannot read property 'length' of undefined
    at Object.exports.appendArrayPointValue (plotly-1.28.2.js:113510)
    at _hover (plotly-1.28.2.js:113987)
    at Object.hover (plotly-1.28.2.js:113608)
    at SVGRectElement.maindrag.onmousemove (plotly-1.28.2.js:138465)

Apologies this isn't a simpler plot type, but I've noticed this while working on groupby and the splinter tests now seem to be failing because of it. I tried and failed to reproduce it with a plain codepen, so this is the first time I've been able to reproduce it in a couple lines.

@rreusser
Copy link
Contributor Author

I don't know for sure, but it appears trace._arrayAttrs is undefined sometimes. That causes a failure on this line. Is the solution to simply return if there are no arrayAttrs?

@etpinard
Copy link
Contributor

Thanks for heads-up.

That codepen you linked is blank. We should add a test before merging your PR

@rreusser
Copy link
Contributor Author

Hahaha fail. https://codepen.io/rsreusser/pen/bRRBjg

@etpinard
Copy link
Contributor

Oh hacky finance charts!

@rreusser
Copy link
Contributor Author

rreusser commented Jun 22, 2017

To be clear, I did see it on other trace types but wasn't able to reproduce it despite copying the JSON verbatim from gd.data in the workspace. I was a bit confused, so I jumped on this reproduction, despite the OHLC.

@etpinard
Copy link
Contributor

I did see it on other trace types but wasn't able to reproduce it despite copying the JSON verbatim from gd.data in the workspace. I was a bit confused, so I jumped on this reproduction, despite the OHL

Ok. In this case, the adding a fallback for undefined _arrayAttrs would be safest and best.

I'll like to get a fix in today, so I'll try to a few test cases to your #1808 PR. Thanks again for spotting this!

@etpinard etpinard added the bug something broken label Jun 22, 2017
@alexcjohnson
Copy link
Collaborator

Ok. In this case, the adding a fallback for undefined _arrayAttrs would be safest and best.

That's the simplest solution... but that'll mean transformed traces will lose access to these attributes, exactly when they're most useful, won't it? Would it be better to recalculate _arrayAttrs after running the transforms?

@etpinard
Copy link
Contributor

@rreusser 's #1808 brought back hover for finance traces, but this issue isn't completely resolved.

Event data for finance traces and, more generally, all transformed traces isn't currently showing all array attribute values as what PR #1770 put forward for non-transformed traces.

Changing the issue title accordingly.

@etpinard etpinard changed the title Hover failing on some trace types Event data for transformed traces is inconsistent Jun 26, 2017
@rreusser
Copy link
Contributor Author

Yeah, I definitely prefer to fix these issues and not have failsafes in place for internal code.

@etpinard
Copy link
Contributor

A quick note about this ticket post-#2162 (see also #2128) where all transforms now compute indexToPoints map objects.

Adding indexToPoints to the ohlc and candlestick transform modules could somewhat easily resolve this ticket here.

That said, I think we should instead spend the time (not more than 1-week) rewriting ohlc and candlestick w/o transforms altogether. Making ohlc and candlestick conventional trace types with there own calc step and thoughtfully reusing Scatter.plot and Box.plot in their plot step would make them a lot less painful to work with (especially in the plotly workspace and for fixing #2004).

@nirvana-msu
Copy link

nirvana-msu commented Dec 29, 2017

I'm facing this issue when I try to use candlestick plot. I'm assigning customdata, to determine which one was clicked on, however the values of customdata on a point in the callback are somewhat in a random order. It seems to be exactly the same issue as was described in #2095. customdata I get corresponds to pointNumer, and both are wrong.

This issue makes it impossible to use such chart as part of the interactive (cross-filtering) workflow in Dash, which is very frustrating.

Is there a usable workaround? This issue has been open for half a year, and I would really like to use this type of chart in Dash...

@etpinard
Copy link
Contributor

I can't think of any workaround at the moment.

PR #2126 (and followup #2162) put forward some foundation work that could be used to somewhat easily resolve this issue. Unfortunately, no plotly.js team member will dedicate time to this issue in the next 2-3 weeks. But if someone is interesting in making a PR, we'll gladly guide them through it.

@etpinard
Copy link
Contributor

will be fixed as part of #2510

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

No branches or pull requests

4 participants