heliosdrm / GRUtils.jl

Tools for using the GR framework in Julia
Other
31 stars 6 forks source link

GRUtils - GR compatibility #45

Open jheinen opened 4 years ago

jheinen commented 4 years ago

First of all: Many thanks for this great package. So far I simply didn't have the time to deal with GRUtils intensively.

I have now tried to use GRUtils completely as a replacement for jlgr. In principle this works fine. But there are still some points to discuss:

Before we take this step - with your permission - some things might need to be revised:

Once again, thank you very much for the great implementation.

I would be happy if we could improve the quality of the GR software in a joint effort. For the users it would be a valuable enrichment.

heliosdrm commented 4 years ago

I fully agree with the idea - since the beginning I was thinking on this repo as a sandbox to experiment with a module that could be eventually merged into GR. So, no problem in moving everything to your repo when the issues are solved. I'll open separate issues for each bullet point. (Also, perhaps the approach to documentation should be revised: my docstrings are made to look good when parsed as markdown - with Documenter.jl -, whereas GR's docstrings are for rst.)

jheinen commented 4 years ago

Great! 👍

As soon as we have the above issues solved - I have to do my part in GR, too - we can start merging. I just set up an experimental environment on my MacBook using Julia 1.5.0-DEV.217 and with the exception of the above points everything looks fine. The "package-compiled" version also works well.

As far as the documentation is concerned, I have to consult my colleague. But we should be able to manage that ...

heliosdrm commented 4 years ago

I do not fully understand the following:

We should add an optional "immediate" mode to (automatically) display plots with each workstation update (updatews).

GR.updatews is now called in the draw method for Figure objects -- and indirectly every time a Figure is shown, e.g. by calling gcf(). I don't know if the automatic display of plots upon workstation updates you refer to is on my side or on yours.

Also:

The (old) examples must be slightly adapted (column-major vs. row-major ordering of arrays). Your examples are already compatible with MPL, MATLAB, R, ...

You are talking about what I commented in https://github.com/jheinen/GR.jl/pull/246, isn't it?

So, do you mean that the change I introduced there (and also in the code of GRUtils) is right and the examples of GR docs must be changed, or that the code of GRUtils should be modified to conform the current criterion of GR?

jheinen commented 4 years ago

I did already change the examples in GR#master. There's no need to change anything in GRUtils.

The new draw method for Figure objects should be invoked when the user starts a script with multiple plot commands (e.g. snoop.jl). I will check how this can be established.

heliosdrm commented 4 years ago

Ok. About invoking draw for every plotting command: the fix would be really easy: change a line of the macro @plotfunction that creates the plotting functions, such that instead of returning a figure they return the output of draw applied to that figure.

That was how the first versions of GRUtils worked, but I thought that it was not good. That method of draw is type-unstable (it returns the output of GR.show or nothing, and what GR.show makes depends on globals). Therefore, drawing the figure is the step that actually takes more time (see #46), and for some workflows it is nice to be able to save that time if you don't want to show the output of every step. Others (cf. #42) have also reported that they prefer skipping the step of showing the plots.

As it is now, one clean workaround for showing intermediate plots in multi-line scripts, is to pipe draw after those lines whose results you want to show. For instance:

using GRUtils
x = LinRange(-2, 2, 40)
y = x.^3 .+ x.^2 .+ x
hold(true)
for s = 1:10
    plot(x, y/s)
    title("scale: 1/$s") |> draw
    sleep(1)
end

This has the advantage of being able to choose which lines you really want to display, and skip the others. (In the example above, the plots are only drawn after adding the title).

If wanted, it might be possible to add variants of the plotting functions adding the draw step. In such case, I'd vote for macros that add it after invoking any function that returns a figure.

jheinen commented 4 years ago

I understand - makes sense.

Perhaps GR and GRUtils can initially be operated in parallel. To do so, we should only address the two compatibility issues (#47, #48).

I am currently doing my tests with snoop.jl from the GR examples. Except for the problems mentioned above, everything works fine.

heliosdrm commented 4 years ago

There may be other compatibility issues to look at. The differences between GRUtils and GR's API are listed here: https://heliosdrm.github.io/GRUtils.jl/stable/#Relationship-with-GR's-API-1

  • All the plotting functions that can take matrices as bi-dimensional inputs (contour, contourf, surface, wireframe, heatmap) consider that X coordinates are mapped to columns and Y to rows of the input matrix.
  • The radius of polar plots always has its centre at zero, instead of the minimum value of the represented data.
  • The angle labels in polar and polarhistogram are by default in radians; and the bins of polarhistogram are by default positioned according to the values of the input.
  • Matrices passed to imshow must contain numbers in the range [0, 1].
  • The function isosurface does not assume a default "isovalue", which has to be entered explicitly as second positional argument.
  • Staircase plots (not present in GR's documentation) are made with the function stair instead of step, in order to avoid name conflicts with Base.step.

Please let me know which of those differences should be revised.

jheinen commented 4 years ago
  • All the plotting functions that can take matrices as bi-dimensional inputs (contour, contourf, surface, wireframe, heatmap) consider that X coordinates are mapped to columns and Y to rows of the input matrix.

I changed this already in GR master.

  • Matrices passed to imshow must contain numbers in the range [0, 1].

It should also be possible to plot RGBA arrays as in GR - in the style of MATLAB.

  • The function isosurface does not assume a default "isovalue", which has to be entered explicitly as second positional argument.

I added an alias in GR master.

  • Staircase plots (not present in GR's documentation) are made with the function stair instead of step, in order to avoid name conflicts with Base.step. I will change the code to avoid the step function.

For the polar plot issues, it would be helpfull to have some examples.

heliosdrm commented 4 years ago

Created #52 and #53 to deal with the issues of polar plots and the interface of imshow separately.