Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: grand constructor unification #44

Closed
tpapp opened this issue Jan 16, 2018 · 2 comments · Fixed by #75
Closed

RFC: grand constructor unification #44

tpapp opened this issue Jan 16, 2018 · 2 comments · Fixed by #75

Comments

@tpapp
Copy link
Collaborator

tpapp commented Jan 16, 2018

Grand constructor unification

This addresses #11, #16, #37, and continues #43.

Objectives

  1. Consistent constructors of the form Type([options], ...) where ... include various convenience forms. Easier to remember, more consistent with the LaTeX code of pgfplots.

  2. Optionally, shorten some struct/constructor names. The expectation is that the user uses pgf. for the module. This is an optional part of the RFC, and the refactoring can be done without it.

  3. Make constructors validate whatever possible (eg dimension consistency etc), to fail early.

Remark: in order to make this writeup more compact, Vector and Matrix stand-in for their abstract counterparts below. The functions should always accept everything.

Specifics

Whenever we have options, it becomes the first field of the struct.

TikzDocument

Rename to Document. Constructor is TikzDocument([elements]; preamble).

TikzDocument(elements::Vector; preamble) is deprecated.

TikzDocument(element) and TikzDocument(; preamble) survive as special cases.

TikzPicture

Rename to Picture. The constructor is TikzPicture([options], elements...).

TikzPicture(elements::Vector, options), TikzPicture(options, elements), and TikzPicture(elements::Vector) are deprecated. TikzPicture(options) remains as a special case.

Axis

The constructor is Axis([options], plots...).

Axis(plots::Vector, options), Axis(plot, options...) are deprecated.

Very similarly: PolarAxis, also introduce SemiLogXAxis, SemiLogYAxis, LogLogAxis.

Plot, Plot3

Introduce Plot([incremental], [options], elements...).

Plot(elements::Vector, options), Plot(elements:Vector, options...), and forms with label are removed.

legend was used for \addlegendentry. But this can be specified alternatively:

  1. with a Legend type which would emit \legend (does not exist currently)
  2. with a LegendEntry type which would emit \addlegendentry (similarly, does not currently exist)
  3. with the legend entries option for Axis.

Furthermore, Section 4.9.4 of the manual says that

It does not matter where \addlegendentry commands are placed, only the sequence matters.

so there is no strong reason to have it in Plot.

Very similarly for Plot3.

Coordinates

Coordinates(coords::Matrix, [error::Matrix], [meta::Vector]) is the single inner constructor. It checks that coords and error have the same size (if applicable), with 2 or 3 columns, and that meta has matching length (when provided).

Convenience constructors could include:

  • Coordinates(x::Vector, y::Vector; [xerror::Vector, yerror::Vector], [meta::Vector]). When an error is given but another is not, the first error is filled with zeros.

  • Coordinates(x::Vector, y::Vector, z::Vector; [meta::Vector]) (I don't think 3d allows error bars, does it?)

  • Coordinates(y::Vector; [meta::Vector]) which makes x a vector of 1, 2, ... (for simple time series etc).

Convenience feature: when a row of coords is missing, an empty line emitted for LaTeX.

Table

Table(options, contents::Matrix, names::Vector) is the default inner constructor. It verifies consistency of names and contents.

Table([options], ::Pair{Union{Symbol, String}, Vector}...) is the preferred constructor for users. Keys can be defined with symbols and strings (all converted to strings). The constructor verifies that lengths match and that there are no duplicate keys.

Convenience constructors:

  • Table([options], x::Vector, [y::Vector], [z::Vector], [meta::Vector]): the first 4 vectors are names x, y, z, and meta automatically when unnamed.

  • Existing converting convenience constructors are kept (eg for StatsBase.Histogram, Contour).

Convenience feature: when any element of a row is missing, an empty line is printed in LaTeX.

Graphics

We keep Graphics([options], filename).

GroupPlot

The new constructor is GroupPlot([options], contents::Matrix).

The group size option is inferred from the dimensions of contents automatically and overwrites any existing option.

contents can have the following elements:

  • a Plot, which is emitted as is with a \nextgroupplot

  • an Axis containing Plots: its options are appended to \nextgroupplot, then the plots are emitted,

  • a missing value: just a \nextgroupplot.

Expression

Unchanged.

Open questions

  1. Should we have a Coordinates3 type separately for 3d coordinates? Does not exist in pgfplots, but the code would be cleaner (I don't think they take error bars, and Plot3 could refuse to accept 2d Coordinates, etc).

  2. Should "key", :key, "key" => value, and :key => value be accepted as a stand-in for options, converting automatically? Advantage: shorter code when using a single option, though using @pgf is not much more verbose. Disadvantage: the constructor for Table would need to disentangle this case, which may be difficult. I would prefer not to do this.

  3. Should we have a Legend and LegendEntry?

Implementation roadmap

  1. Replace type and constructors one by one, to make review easier.

    a. each one gets documented,
    b. unit tests with 100% coverage for new forms,
    c. deprecations are introduced for older forms.

  2. Documentation, including examples, are rewritten extensively.

  3. Release is tagged.

  4. Deprecated forms removed in next release.

@KristofferC
Copy link
Owner

Some comments after a quick readthrough.

  • Regarding renaming of structs, e.g TikzDocument -> Document I am kinda negative. Naming the structs the same as they are in .tex \begin{tikzdocument} makes it easy to know what they refer to. Calling it just Document does not bring the same connotations.

  • Regarding Document(elements...; preamble) or Document([elements]; preamble) I am not sure. I kinda like using a vector because it makes it more likely you realize that you can push! more elements into a Document.

  • Regarding failing early. Agreed, this should always be done.

In general, the fewer structs we have to introduce, the better. If we really have to introduce Legend, Option, LegendEntry etc. types we can, but I really prefer not to.

@tpapp
Copy link
Collaborator Author

tpapp commented Jan 18, 2018

Thanks for the comments. I agree about shortening names, I want to mirror pgfplots names as closely as feasible in order to economize on documentation 😄

This is what I would propose: I rework

  1. Coordinates (splitting 3d into Coordinates3, I think it is cleaner)
  2. Table
  3. Plot
  4. Axis

in this order, making a PR for each separately. These are the ones I care most about, and would address #37 and #11.

The rest we will see after this. I agree about avoiding unnecessary vocabulary if we can avoid it.

tpapp added a commit that referenced this issue Feb 3, 2018
This is a step towards fixing #44, and is in accordance with the
principles laid out there, but not the exact syntax proposed there
since it was found to be impractical. Also fixes #11.

1. Introduce the `Coordinate` type for individual coordinates. Validate
arguments and fail early.

2. Introduce `EmptyLine` for jumps/scanlines. This addresses #11.

3. Rewrite `Coordinates` to contain the above. (`<: OptionType` was
removed, because it has no options).

4. Convenience constructors for creating coordinates from vectors,
matrices, iterable objects, with or without error bars. Docstrings for
the API.

5. Add examples of all the new syntax to the gallery (some of this
should be moved to the documentation eventually, that will come later
after other changes).

6. 2D histograms now work (example will be added later, when `Table`
are redesigned similarly).
KristofferC pushed a commit that referenced this issue Feb 6, 2018
* Rework coordinates.

This is a step towards fixing #44, and is in accordance with the
principles laid out there, but not the exact syntax proposed there
since it was found to be impractical. Also fixes #11.

1. Introduce the `Coordinate` type for individual coordinates. Validate
arguments and fail early.

2. Introduce `EmptyLine` for jumps/scanlines. This addresses #11.

3. Rewrite `Coordinates` to contain the above. (`<: OptionType` was
removed, because it has no options).

4. Convenience constructors for creating coordinates from vectors,
matrices, iterable objects, with or without error bars. Docstrings for
the API.

5. Add examples of all the new syntax to the gallery (some of this
should be moved to the documentation eventually, that will come later
after other changes).

6. 2D histograms now work (example will be added later, when `Table`
are redesigned similarly).

* Fixed missing packages in REQUIRE.

* small formatting changes
This was referenced Feb 7, 2018
tpapp added a commit that referenced this issue Feb 15, 2018
This is a step towards fixing #44 (almost there!).

1. New user-facing plot/plot3 constructor is

```julia
Plot([incrementa], [options], data, trailing...)
```
and similarly for `Plot3`.

`incremental` defaults to `false`, as this seems to be the most common
use case.

Some validation is done on `data` (checking for type).

This removes the `label` kwarg, and allows us to close #16. Examples
now recommend an explicit `\addlegendentry`.

Docstrings are added for everything, and examples are rewritten
accordingly.

2. Axis-like code cleaned up a bit with macros. Variations on log axes
added. Explicitly document that strings are emitted as is.

3. GroupPlot rewritten, allow multiple plots and empty \nextgroupplot,
two examples added, one from the manual and one for multiple plots.

4. Replaced random examples with deterministic ones, perhaps stick to
this and close #63? Minor indentation issues fixed.

5. Minor fixes for testing framework.
KristofferC pushed a commit that referenced this issue Feb 21, 2018
* Rewrite Plot and axis-like types.

This is a step towards fixing #44 (almost there!).

1. New user-facing plot/plot3 constructor is

```julia
Plot([incrementa], [options], data, trailing...)
```
and similarly for `Plot3`.

`incremental` defaults to `false`, as this seems to be the most common
use case.

Some validation is done on `data` (checking for type).

This removes the `label` kwarg, and allows us to close #16. Examples
now recommend an explicit `\addlegendentry`.

Docstrings are added for everything, and examples are rewritten
accordingly.

2. Axis-like code cleaned up a bit with macros. Variations on log axes
added. Explicitly document that strings are emitted as is.

3. GroupPlot rewritten, allow multiple plots and empty \nextgroupplot,
two examples added, one from the manual and one for multiple plots.

4. Replaced random examples with deterministic ones, perhaps stick to
this and close #63? Minor indentation issues fixed.

5. Minor fixes for testing framework.

* Separate incremental plot constructors, add LegendEntry.

The `incremental` flag was removed from `Plot*` constructors in the
API, replaced by `PlotInc` and `Plot3Inc`.

A LegendEntry type was added.

Minor fixes:

- remove the INCREMENTAL constant, as it is now unnecessary

- clarify docstrings

- disabled docstring checks, as they lead to infinite loops for some
  reason

* Trivial docstring fix.
tpapp added a commit that referenced this issue Mar 6, 2018
This completes and closed #44.

`TikzPicture([options], elements...)` is the new syntax, consistently
with the other `OptionType`s. Fix examples in documentation.

`TikzDocument(elements...; ...)` is the new syntax. The default
preamble is obtained dynamically from the global variables, not at the
time of construction as before.
@tpapp tpapp closed this as completed in #75 Mar 6, 2018
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 a pull request may close this issue.

2 participants