refactoring the plotting system #166

wpfff · 2020-12-03T02:32:03Z

some first working bits and pieces for slightly restructured plotting code.
main aspects:

separate all parts that are not backend (like matplotlib) dependent but more general. This includes quite a lot of the current logic in the autoplot elements.
should end up such that adding other plotting backends should be fairly easy.

Some notes for orientation:

there's no more mpl.py file but rather a submodule
included a simple way for customizing matplotlib appearance using a config system with plottrconfig_*.py file
added a new class 'AutoFigureMaker' whose job is only to track the elements of the figure and arrange them into subplots depending on user settings.
2d plots are not yet working, but it should be enough to get an idea of where it's going.

… imports.

Added new class that is supposed to handle arranging plots into different panels (no need to mix that with actual plotting code).

…nly 1D plots work, rest is non-functional at this point.

wpfff · 2020-12-03T02:34:57Z

@jenshnielsen
this is still very rough, but i thought i'll open a PR already to give you chance to look since you also wanted to make changes to the matplotlib parts.

some tests are working fine, which hopefully should be enough to get an idea, see:
/test/gui/mpl_autoplot.py
/test/apps/autoplot_app.py

(2D plots i need to plug back in -- i stopped today when the main parts seemed to fit together)

would be great to hear any thoughts!

wpfff · 2020-12-03T02:36:32Z

i should also add that the recent changes for high-dpi scaling aren't yet re-implemented here. but shouldn't be too hard to get that merged back in.

astafan8 · 2020-12-04T13:28:28Z

plottr/plot/base.py

+            if len(plotItem.data) == 2:
+                style = 'line'
+            elif len(plotItem.data) == 3:
+                style = 'image'


what's the difference between this "style" and "PlotType" enum? also, shouldn't determinePlotDataType be used here somehwere?

As I understand it style is basically:

1D or 2D

PlotDataType and PlotType are further refinements of this.

Perhaps we should consider if we can streamline these types. My understanding is basically that we can categorize the data as follows. I like to think of it as a mapping from data types to valid plot types

1d data ordered: [1D line plot, 1D points plot (scatter)]

1d data not ordered: [1D points plot (scatter)]

2d data on equidistant grid: [image, colormesh, scatter2d]

2d data on non equidistant grid: [colormesh, scatter2d]

2d data no structure: [scatter2d]

Further to that there is choice between single and multitrace 1d plots which seems to be somewhat unrelated.

it seems to me that these types currently mix things that should be separate concerns but since that code is the same as it was before in mpl.py we should probably not touch that in this pr

good point, Mikhail. this could essentially be the same thing. the idea is that any backend could essentially support different types of plots that for instance also the user may want to implement (i'm thinking of a few custom plots that would be useful for specific measurements). Other backends might support 3D graphics in a decent way. etc...

Jens, agreed. The only thing i'd want to make sure of is that adding plot types should be somewhat easy, and potentially doable through a plugin rather than only by modifying 'core' code.

I'm definitely open to come up with a structure that does things 'right' in this effort, in terms of separation. There's no immediate rush with merging this into the main branch.

plottr/plot/base.py

astafan8 · 2020-12-04T13:31:37Z

plottr/plot/mpl/__init__.py

+    """Set the matplotlib style based on available config files."""
+    styleFiles = configFiles('plottr_default.mplstyle')
+    if len(styleFiles) > 0:
+        plt.style.use(styleFiles[0])


if there are more the one files, then perhaps they should be applied in a "hierarchical sum" like, say, git config files?

yes, that'd be great. i have to mess around with rc params in matplotlib a bit to see how that could work.

So far I don't see an easy way with the matplot methods directly.
I'm thinking of including this into a central config file for plottr that will also configure matplotlib parameters.

i pushed a proposal for a config system with just python files (so no other specific file format) that both supports the hierarchical sum concept you mentioned, and can do all configuration of matplotlib.

plottr/plot/mpl/autoplot.py

jenshnielsen

@wpfff I think this looks promising. There is a bit of work to ensure feature parity with the existing code and some cleanup but I like the general direction

plottr/plot/base.py

jenshnielsen · 2020-12-04T14:09:16Z

plottr/plot/base.py

+            if len(plotItem.data) == 2:
+                style = 'line'
+            elif len(plotItem.data) == 3:
+                style = 'image'


As I understand it style is basically:

1D or 2D

PlotDataType and PlotType are further refinements of this.

Perhaps we should consider if we can streamline these types. My understanding is basically that we can categorize the data as follows. I like to think of it as a mapping from data types to valid plot types

1d data ordered: [1D line plot, 1D points plot (scatter)]

1d data not ordered: [1D points plot (scatter)]

2d data on equidistant grid: [image, colormesh, scatter2d]

2d data on non equidistant grid: [colormesh, scatter2d]

2d data no structure: [scatter2d]

Further to that there is choice between single and multitrace 1d plots which seems to be somewhat unrelated.

it seems to me that these types currently mix things that should be separate concerns but since that code is the same as it was before in mpl.py we should probably not touch that in this pr

wpfff · 2020-12-04T15:49:29Z

Thanks for the feedback, both of you! I'll probably have some time this weekend to ponder some of the deeper suggestions and see if i can refine the concept a bit (especially the role of the PlotType, style, etc.).

# Conflicts: # plottr/__init__.py # plottr/plot/mpl.py

- PlotType is not a global concept, but something only the MPL plot uses - PlotItem.style is replaced with type PlotDataType to streamline - make no assumptions about plotting, leave all details to the backend classes

- PlotWidgets obtain data from the Nodes, and process UI options - based on UI options, data is processed and passed to FigureMaker - No actual plotting happens in the Widget, this is the role of the FigureMaker

deleted old templates.

- switch to configuration method that allows hierarchical resolution of user configuration (pure python, because that seems the best solution for configuring matplotlib)

FarBo · 2021-05-03T12:44:52Z

@wpfff
I see copy metadata is added. Legend seems behaving fine.
Still the plottype buttons are very small compared to the latest release:
https://1drv.ms/u/s!AkKmOu47nt6ygrI_W8T8SJUw_AHZFQ?e=ArafkO

wpfff · 2021-05-03T12:56:42Z

Still the plottype buttons are very small compared to the latest release:
https://1drv.ms/u/s!AkKmOu47nt6ygrI_W8T8SJUw_AHZFQ?e=ArafkO

@FarBo : true -- matter of preference I guess. Along with one of my recent commits I made them the same size as the ones in the other toolbar to be consistent which i thought looked better (they are dpi-scaled, though).
I'm open to changing that back, though, if you disagree.

FarBo · 2021-05-03T13:07:23Z

@wpfff
true -- matter of preference I guess. Along with one of my recent commits I made them the same size as the ones in the other toolbar to be consistent which i thought looked better (they are dpi-scaled, though). I'm open to changing that back, though, if you disagree.

The buttons on the right looks natural to me and as far as I remember, small sizes (left) are not preferred by some of users.
I think it would be nice to have the bigger like the right plot.

FarBo · 2021-05-11T07:45:58Z

@wpfff
Thanks for the updates. I didn't go through the code structure and only tested the functionality. These final commits solved all my raised concerns. :)

wpfff · 2021-05-12T03:19:52Z

@wpfff
Thanks for the updates. I didn't go through the code structure and only tested the functionality. These final commits solved all my raised concerns. :)

Thanks, @FarBo! I'm still waiting for comments from @astafan8 (for this PR i think it'd be important that there's some consensus on structure, and he wanted to have a look)

wpfff · 2021-05-22T13:42:31Z

@astafan8 - just to poke you :) are you still planning on giving comments on this?
If not, i'd like to merge soon. It's quite well-behaved now and i'm working on an additional backend that follows this structure.
There will probably still be some refactoring once i have another backend going (mainly regarding what should be part of the matplotlib specific part and what is more general) but the overall structure works well i think.

astafan8 · 2021-05-25T09:52:44Z

just to poke you :) are you still planning on giving comments on this?

thanks @wpfff ! I will do this week

astafan8 · 2021-05-31T07:59:59Z

@wpfff ETA today :)

astafan8

left a few notes/comments but let's merge this finally :) i'm happy to tag a release tomorrow

plottr/__init__.py

plottr/plot/base.py

plottr/plot/mpl/widgets.py

astafan8 · 2021-05-31T19:20:17Z

plottr/plot/mpl/autoplot.py

+        assert len(plotItem.data) == 2
+        lbl = plotItem.labels[-1] if isinstance(plotItem.labels, list) and len(plotItem.labels) > 0 else ''
+        x, y = plotItem.data
+        return axes[0].plot(x, y.real, label=lbl, **plotItem.plotOptions)


what is .real really needed for? isn't the data already split into real/imag/mag/phase parts? same in the plotImage method

Suggested change

return axes[0].plot(x, y.real, label=lbl, **plotItem.plotOptions)

return axes[0].plot(x, y, label=lbl, **plotItem.plotOptions)

good point -- this looks wrong (although there was still a -- hidden -- possibility that data might be complex typed).
i fixed it in the correct spot (apparently numpy.ma.abs can produce complex valued data).

wpfff · 2021-06-01T15:24:31Z

Thanks, @astafan8 -- i'll adopt your suggestions today.
I'll probably also include the importlib suggestion. Much better solution -- i just hadn't looked properly :)

…ented.

Co-authored-by: Mikhail Astafev <[email protected]>

wpfff added 11 commits June 23, 2020 08:56

started working on better plot system.

c76da0e

merge from main repo.

154f909

added some basic functionality to find config files.

27462a3

small fix: allow passing of keyword arguments from the command line.

3f18cdd

[WIP] started to move matplotlib helper functions around.

491a128

fix: don't import from lower level module on import to avoid circular…

ea93db1

… imports.

added stylefile where we can easily find it.

08a0025

moved things up a level that are basically plotting backend-independent.

fa6c6f8

Added new class that is supposed to handle arranging plots into different panels (no need to mix that with actual plotting code).

moved widgets that use MPL but are higher-level than autoplot into here.

80ee2ac

[WIP] first steps and in modified autoplot using matplotlib. so far o…

0cc7b6b

…nly 1D plots work, rest is non-functional at this point.

merge from remote master.

97a9230

wpfff requested a review from jenshnielsen December 3, 2020 02:32

astafan8 reviewed Dec 4, 2020

View reviewed changes

plottr/plot/base.py Outdated Show resolved Hide resolved

astafan8 reviewed Dec 4, 2020

View reviewed changes

plottr/plot/mpl/autoplot.py Show resolved Hide resolved

astafan8 reviewed Dec 4, 2020

View reviewed changes

plottr/plot/mpl/autoplot.py Outdated Show resolved Hide resolved

jenshnielsen reviewed Dec 4, 2020

View reviewed changes

wpfff added 9 commits January 3, 2021 10:41

Merge branch 'master' into betterplotting

fabaa76

Merge branch 'master' into betterplotting

e92bd1b

# Conflicts: # plottr/__init__.py # plottr/plot/mpl.py

some better organization:

990c00f

- PlotType is not a global concept, but something only the MPL plot uses - PlotItem.style is replaced with type PlotDataType to streamline - make no assumptions about plotting, leave all details to the backend classes

clarifying better which objects are responsible for what:

29d1e9b

- PlotWidgets obtain data from the Nodes, and process UI options - based on UI options, data is processed and passed to FigureMaker - No actual plotting happens in the Widget, this is the role of the FigureMaker

re-enabled basic colorplotting.

3f589a1

deleted old templates.

fixed type hints in plotting.py

d79fcbf

fixing more typing annotations for mypy.

45ee82d

more cleanup for mypy.

6d845e1

more cleanup for mypy.

c7669d1

wpfff added 3 commits April 30, 2021 08:48

make sure meta entries don't get too long when copying.

acb0e8d

don't inherit from object.

a6a69f9

- added central method for scaling dpi

f01cf40

- switch to configuration method that allows hierarchical resolution of user configuration (pure python, because that seems the best solution for configuring matplotlib)

wpfff requested a review from FarBo May 3, 2021 12:32

wpfff added 3 commits May 10, 2021 19:45

change back to bigger icons in the toolbar

5523567

fixed a couple of mypy issues.

1ec2d6a

more mypy fixing.

a792928

FarBo approved these changes May 11, 2021

View reviewed changes

Add disappeared "imported plottr version" log message

2ab1ca6

astafan8 approved these changes May 31, 2021

View reviewed changes

astafan8 removed the request for review from jenshnielsen June 1, 2021 15:20

Merge branch 'master' into betterplotting

90e3dc8

wpfff and others added 7 commits June 3, 2021 10:11

importing config files without modifying sys.path.

a9ef0f0

clarify why formatSubPlot does not raise an error when not reimplem…

6f3cb98

…ented.

fix mypy issue with importlib.

1d09196

Update plottr/plot/mpl/widgets.py

c12e508

Co-authored-by: Mikhail Astafev <[email protected]>

enforcing real in a more sane spot.

5359ee4

Merge remote-tracking branch 'origin/betterplotting' into betterplotting

8622f9c

fix typing.

ba44f82

wpfff merged commit e648047 into toolsforexperiments:master Jun 4, 2021

wpfff deleted the betterplotting branch June 4, 2021 03:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactoring the plotting system #166

refactoring the plotting system #166

wpfff commented Dec 3, 2020 •

edited by astafan8

Loading

wpfff commented Dec 3, 2020

wpfff commented Dec 3, 2020

astafan8 Dec 4, 2020

jenshnielsen Dec 4, 2020

wpfff Dec 4, 2020

astafan8 Dec 4, 2020

wpfff Dec 4, 2020

wpfff Apr 30, 2021

wpfff May 3, 2021

jenshnielsen left a comment

jenshnielsen Dec 4, 2020

wpfff commented Dec 4, 2020

FarBo commented May 3, 2021 •

edited

Loading

wpfff commented May 3, 2021

FarBo commented May 3, 2021

FarBo commented May 11, 2021 •

edited

Loading

wpfff commented May 12, 2021

wpfff commented May 22, 2021

astafan8 commented May 25, 2021

astafan8 commented May 31, 2021

astafan8 left a comment

astafan8 May 31, 2021

wpfff Jun 4, 2021

wpfff commented Jun 1, 2021

	return axes[0].plot(x, y.real, label=lbl, **plotItem.plotOptions)
	return axes[0].plot(x, y, label=lbl, **plotItem.plotOptions)

refactoring the plotting system #166

refactoring the plotting system #166

Conversation

wpfff commented Dec 3, 2020 • edited by astafan8 Loading

wpfff commented Dec 3, 2020

wpfff commented Dec 3, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jenshnielsen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wpfff commented Dec 4, 2020

FarBo commented May 3, 2021 • edited Loading

wpfff commented May 3, 2021

FarBo commented May 3, 2021

FarBo commented May 11, 2021 • edited Loading

wpfff commented May 12, 2021

wpfff commented May 22, 2021

astafan8 commented May 25, 2021

astafan8 commented May 31, 2021

astafan8 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wpfff commented Jun 1, 2021

wpfff commented Dec 3, 2020 •

edited by astafan8

Loading

FarBo commented May 3, 2021 •

edited

Loading

FarBo commented May 11, 2021 •

edited

Loading