Skip to content

Matplotlib exporter scientific notation bugfix#1051

Closed
penkod wants to merge 2 commits intopyqtgraph:developfrom
penkod:develop
Closed

Matplotlib exporter scientific notation bugfix#1051
penkod wants to merge 2 commits intopyqtgraph:developfrom
penkod:develop

Conversation

@penkod
Copy link
Copy Markdown

@penkod penkod commented Oct 1, 2019

Fix setting labels and values in case of plots which use scientific notation.

Solves issue #1050

Copy link
Copy Markdown
Member

@ixjlyons ixjlyons left a comment

Choose a reason for hiding this comment

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

I think I would prefer fixing the labels to actually scaling the data. I kind of alluded to that in #1050 , but I suppose this works and requires fewer code changes. The only issue I see is that no information about the scale of the data is not provided to matplotlib, aside from the axis label. I'm inclined to accept this PR though if you don't mind working on the specific things I commented on.

for item in self.item.curves:
x, y = item.getData()

# pyqtgraph by default uses scientific notation for large
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just one copy of the comment at the top should be sufficient.


# pyqtgraph by default uses scientific notation for large
# values. Matplotlib values and label should reflect that too.
SIprefix_scale_default = 1.0 # default SI prefix scale value
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think a more straightforward implementation might loo something like:

scale_x = scale_y = 1.0
if self.items.axes['bottom']['item'].autoSIPrefix: # note no need for == True here
    scale_x = self.item.axes['bottom']['item'].autoSIPrefixScale
if self.items.axes['left']['item'].autoSIPrefix:
    scale_y = self.item.axes['left']['item'].autoSIPrefixScale

Then simply apply scale_x/scale_y below without the conditionals.

@ixjlyons
Copy link
Copy Markdown
Member

ixjlyons commented Nov 9, 2019

Hey @penkod, I can go ahead and build from your commits to implement the requested changes. If you prefer to do it yourself, let me know. Thanks for the contribution in any case.

@penkod
Copy link
Copy Markdown
Author

penkod commented Nov 11, 2019

Hi @ixjlyons. At the time being, I unable to do the necessary changes + the tests, so of course, if you wish you can implement the necessary changes yourself.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants