Skip to content

Implemented PlotDataItem.appendData()#850

Closed
awkbr549 wants to merge 2 commits intopyqtgraph:masterfrom
awkbr549:develop
Closed

Implemented PlotDataItem.appendData()#850
awkbr549 wants to merge 2 commits intopyqtgraph:masterfrom
awkbr549:develop

Conversation

@awkbr549
Copy link
Copy Markdown

@awkbr549 awkbr549 commented Apr 13, 2019

Simple implementation of PlotDataItem.appendData() based on PlotDataItem.setData().

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented May 24, 2019

Thanks for the PR; would you mind adding some test code to ensure that this method behaves as intended? The tests should go in pyqtgraph/graphicsItems/tests/test_PlotDataItem.py

@j9ac9k j9ac9k self-assigned this Aug 18, 2019
@j9ac9k j9ac9k changed the base branch from develop to master October 15, 2020 04:08
j9ac9k
j9ac9k reviewed Oct 15, 2020
View reviewed changes
pyqtgraph/graphicsItems/PlotDataItem.py
profiler('update items')
return
if y is not None and x is None:
x = np.arange(len(y))
Copy link
Copy Markdown
Member

@j9ac9k j9ac9k Oct 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just playing with this via the ViewBox example, and I don't think this should be correct;

in the ViewBox example, adding this line:

l1.appendData(np.flip(y))

will result in this

image

If appending the data, I would imagine it would continue the x-axis from where it left off...

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Oct 15, 2020

Looking over this diff, looks like most of the code is just copied from the setData() method; I'm good with implementing this feature, but we shouldn't have so much duplicated code, and should refactor out much of it into other methods to parse the input arguments at the very least.

@j9ac9k j9ac9k mentioned this pull request Nov 7, 2020
Open
@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Nov 7, 2020

I'm closing out this issue, but I created #1430 as I do think this is a good feature to add, so progress ideally can be tracked there.

Thanks again for the PR @awkbr549 if you can address the issues I brought up, feel free to reopen.

@j9ac9k j9ac9k closed this Nov 7, 2020
@NilsNemitz NilsNemitz mentioned this pull request Nov 21, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@j9ac9k j9ac9k j9ac9k left review comments

Assignees

@j9ac9k j9ac9k

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@awkbr549 @j9ac9k