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

fig.append_trace() raises ValueError if row arg is not type int. #2391

Closed
mark-datanaltek opened this issue Apr 16, 2020 · 7 comments
Closed
Labels
bug something broken good first issue suitable for newcomers

Comments

@mark-datanaltek
Copy link

fig.append_trace() raises ValueError if the row arg is not type int, even if arg is int-like:
DEBUG row:1, type(row):<class 'numpy.int64'>

If I explicitly cast to int with row=int(row), all is well. The issue may be due to the isinstance check at line 1536 of basedatatypes.py?

/usr/local/lib/python3.7/site-packages/plotly/basedatatypes.py in _validate_rows_cols(name, n, vals)
1535
1536 if [r for r in vals if not isinstance(r, int)]:
-> 1537 BaseFigure._raise_invalid_rows_cols(name=name, n=n, invalid=vals)
1538 else:
1539 BaseFigure._raise_invalid_rows_cols(name=name, n=n, invalid=vals)

/usr/local/lib/python3.7/site-packages/plotly/basedatatypes.py in _raise_invalid_rows_cols(name, n, invalid)
1524 )
1525
-> 1526 raise ValueError(rows_err_msg)
1527
1528 @staticmethod

ValueError:
If specified, the rows parameter must be a list or tuple of integers
of length 1 (The number of traces being added)

    Received: [1]

I am working with the latest version:

import plotly
plotly.version
'4.6.0'

@emmanuelle
Copy link
Contributor

Thank you @mark-datanaltek this is indeed a bug. To reproduce

import numpy as np
import plotly.graph_objects as go
indices_rows = np.array([0], dtype=np.int)
indices_cols = np.array([0], dtype=np.int)
fig = go.Figure()
fig.add_trace(go.Scatter(y=[1]), row=indices_rows[0], col=indices_cols[0])
fig.show()

@emmanuelle emmanuelle added the bug something broken label May 1, 2020
@emmanuelle
Copy link
Contributor

This could be easily fixed by modifying slightly this line of code https://github.com/plotly/plotly.py/blob/master/packages/python/plotly/plotly/basedatatypes.py#L1531
Since numpy is not a dependency we would have to check whether numpy is imported

try:
    import numpy as np
    int_type = (int, np.integer)
except ImportError:
    int_type = (int,)
....
if [r for r in vals if not isinstance(r, int_type)]:
....

@emmanuelle emmanuelle added the good first issue suitable for newcomers label May 1, 2020
@MCBoarder289
Copy link
Contributor

Thanks @emmanuelle for tagging this as a good first issue. I'll see if I can give it a try here!

@MCBoarder289
Copy link
Contributor

Ran into an exception with the reproducing example once I got this working:

Exception: In order to reference traces by row and column, you must first use plotly.tools.make_subplots to create the figure with a subplot grid.

Here is one that works with a fix in place. Seeing if I can write this into a test before creating a pull request:

import numpy as np
import plotly.graph_objects as go
from plotly.subplots import make_subplots
indices_rows = np.array([1], dtype=np.int)
indices_cols = np.array([1], dtype=np.int)
fig = make_subplots(rows=1, cols=1)
fig.add_trace(go.Scatter(y=[1]), row=indices_rows[0], col=indices_cols[0])

@MCBoarder289
Copy link
Contributor

I have put a pull request up here: #2451
Open to any feedback on this!

@MCBoarder289
Copy link
Contributor

@emmanuelle Thanks for the review! Should we link #2451 now that's been approved and merged for this ticket? I wasn't sure how to do that when I made the PR.

@emmanuelle
Copy link
Contributor

@MCBoarder289 you can write "closes #xx" with the issue number when you start a PR, then the issue will be closed automatically on merging the PR.

Closing since this has been closed by #2451.

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

No branches or pull requests

3 participants