Repository navigation
Edge cases in eventplot are likely broken #7560
Description
The eventplot code currently contains the snippet
if len(lineoffsets) == 0:
lineoffsets = [None]
if len(linelengths) == 0:
linelengths = [None]
if len(linewidths) == 0:
lineoffsets = [None] # oops
if len(linewidths) == 0:
lineoffsets = [None] # oops
if len(colors) == 0:
colors = [None]
which is certainly incorrect.
Activity
dstansby commentedon Dec 10, 2016
For reference, some code that currently fails:
tacaswell commentedon Dec 10, 2016
It is not obvious that this is wrong, these values get passed into an
EventCollection__init__which may use it's default in place of theNone(but I have not chased down the code path to see if it actually happens).oscargus commentedon Jan 22, 2022
I hijack this issue as this is also an edge case.
Is
eventplot(1)supposed to work?It is handled in the code:
matplotlib/lib/matplotlib/axes/_axes.py
Lines 1263 to 1264 in 1791022
But breaks later since the content is not a 1D-array.
Replacing the second line with
positions = [np.asanyarray([positions])]makes it work, but maybe there should be an error already there?The docs says:
matplotlib/lib/matplotlib/axes/_axes.py
Lines 1176 to 1177 in 1791022
Also, (related from the perspective that I am trying to increase test coverage) these two lines seems impossible to trigger:
matplotlib/lib/matplotlib/axes/_axes.py
Lines 1270 to 1271 in 1791022
oscargus commentedon Jan 22, 2022
Considering the original issue, #7602 fixed the copy-paste-error, but was not considered a suitable solution.
What about just deleting those lines? Then there will be an error for empty (not the same length), which is fair enough. Or error already when detected that they are empty? (As they actually do not have to be the same length, just not empty.)
timhoffm commentedon Jan 22, 2022
I'd say no. It's not documented to work, it does not work and nobody has complained that it should. Making it work would need additional code, documentation and tests. So let's not jump extra hoops here.
github-actions commentedon Apr 3, 2023
This issue has been marked "inactive" because it has been 365 days since the last comment. If this issue is still present in recent Matplotlib releases, or the feature request is still wanted, please leave a comment and this label will be removed. If there are no updates in another 30 days, this issue will be automatically closed, but you are free to re-open or create a new issue if needed. We value issue reports, and this procedure is meant to help us resurface and prioritize issues that have not been addressed yet, not make them disappear. Thanks for your help!
QuLogic commentedon Apr 4, 2023
There is an open PR for this #22286; I guess a decision needs to be made there, so I added it to the agenda.