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

Plotly/PlotlyJS behavior when linecolor is a vector #1325

Open
wkearn opened this issue Dec 17, 2017 · 3 comments
Open

Plotly/PlotlyJS behavior when linecolor is a vector #1325

wkearn opened this issue Dec 17, 2017 · 3 comments

Comments

@wkearn
Copy link
Contributor

wkearn commented Dec 17, 2017

I haven't fixed rgba_string here:

:color => rgba_string(series[:linecolor]),

because I'm not entirely sure what

plot(randn(10,2),color=[:blue,:red])

should do. This will currently still cause an error for Plotly/PlotlyJS.

GR just outputs two blue lines:

plots1306-4

If I try to apply my previous fix (#1324) to this line, you end up with

plots1306-3

Note that

  1. This is different from what GR gives
  2. The colors here are not blue and red, nor are they the default colors. I'm not entirely sure where they're coming from.
@apalugniok
Copy link
Member

apalugniok commented Dec 17, 2017

Sorry I misunderstood the issue so I deleted my previous comment which no longer made sense 😆

EDIT: I think it should either error or give what GR currently gives. Since you can't really color a line elementwise. You could stop the error by doing _cycle(series[:linecolor],1) which I think would be the best solution.

@mkborregaard
Copy link
Member

I think this should ideally error with an informative error message asking the user if maybe they wanted to pass a 1-row Matrix

@wkearn
Copy link
Contributor Author

wkearn commented Dec 18, 2017

I would tend to agree that this should error helpfully. Rather than updating each of the backends to error where Plotly currently does, it's probably a better idea to fail earlier in the pipeline when we can first detect that linecolor is a column rather than a row vector. I'll look into that and come up with a pull request.

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

Successfully merging a pull request may close this issue.

4 participants