Add marker plot key

[tomgroenwoldt]

Hey! First things first, I really enjoyed working with this crate! Thanks for your effort.

This PR adds another PlotKey, namely the mark=... key. I had the following problem:
I wanted to add multiple plots to one graph, but by default all points of the plots had the same symbol:

While I was able to use your PlotKey::Custom(...) to achieve my desired output below, I quickly found myself adding an own enum for this and thought that your crate would benefit of it as well.

Desired output:

Additionally I added MarkerOption to set the mark options=... key. This way we can control the color and the scaling of a marker:

What do you think about adding this to your crate? If you approve I'm happy to add this to your documentation! :)

[DJDuque]

Thanks a lot for your work. This is something that I am definitely interested in including.

I have left some comments on things that I am not sure how to deal with.

[DJDuque]

I think that there are 2 variants missing here: Asterisk and Text(String).

Also, do you think that it would be good to have a more explicit documentation on how each marker will look like? It could be for example:

/// Set the marker to ⊕.
OPlus,
/// Same as `[Marker::OPlus]` but filled with a color.
OplusFilled

[tomgroenwoldt]

That's a great suggestion, will do :)

[DJDuque]

I think that using a String to define a color is as much of a hack as using the PlotKey::Custom.

What do you think about something like (with a better naming):

// See complete list in section 4.6.4 of the PGFPlots manual version 1.4.1
pub enum PredefinedColor {
    Red,
    Green,
    ....,
    Violet,
}

struct Color {
    // A way to mix colors...
    // We could have a constructor from `PredefinedColor`
    // Other constructor that does e.g. `red!20!white`
    // And another constructor that does e.g. `rgb,255:red,231;green,84;blue,121`
}

[tomgroenwoldt]

That's true, I will look into this. What do you think of using the crate strum for an easy Display derive for enums? We can do something like this:

#[derive(Clone, Copy, Debug, strum::Display)]
#[strum(serialize_all = "lowercase")]
pub enum PredefinedColor {
    Red, // "red"
    Green, // "green"
    Blue, // ...
    Cyan,
    Magenta,
    Yellow,
    Black,
    Gray,
    White,
    Darkgray,
    Lightgray,
    Brown,
    Lime,
    Olive,
    Orange,
    Pink,
    Purple,
    Teal,
    Violet,
}

[DJDuque]

First of all, thanks again for your continued work here, I am very happy with the direction we are going.

I have left some comments and some notes to myself.

[DJDuque]

Note to self:
Maybe this could be pub use instead. Not sure though.
It could also even be pub use in the root lib.rs (?).

[tomgroenwoldt]

In my opininion pub use inside src/axis/plot.rs seems logical, as the user can reference color via use pgfplots::axis::plot::Color right? If you want to restrict the usage of this color to plots only, this would be the way to go I guess. If not just tell me and I'm happy to shift things around :)

[DJDuque]

Maybe we could do something similar to what happens in Plot2D::add_key to get rid of duplicate keys (by discriminant).

The behavior of the plot is exactly the same, but the standalone_string() is just cleaner if we don't re-write keys that are going to get overwritten anyways.

[tomgroenwoldt]

Do you mean duplicate keys inside the Vec<MarkOption> input or generally speaking duplicate Markers in the plot styling?

[DJDuque]

Looking through strums docs, it looks like we could just add #[strum(serialize_all = "lowercase")] and then just overwrite the filled variants.

[tomgroenwoldt]

True, that's a good point. I removed unnecessary macros.

[DJDuque]

Note to self:
Unless I find a source for all possible MarkOptions, this enum should be non_exhaustive (this allows to add more variants without needing a major version bump).

If I don't find a list with all options (this is the most likely scenario), add at least the XScale(f64) and YScale(f64) variants. I will look through some more of my plots to see what other weird keys I have used before.

[tomgroenwoldt]

I added the non_exhaustive macro and those two specific options.

[DJDuque]

Maybe we could replace this method by impl From<PredefinedColor> for Color. Or we could have both, where one just calls the other; I am OK with ether way.

[tomgroenwoldt]

I replaced it, thanks for the suggestion!

[DJDuque]

I just tested this, and I don't think it does exactly what one would expect (at least it threw me off a little bit). You can try the following plot:

\documentclass{standalone}
\usepackage{pgfplots}
\begin{document}
\begin{tikzpicture}
	\begin{axis}[
		]
		\addplot[mark=*, fill=red] coordinates {(0,0.92) (1,0.92)};
		\addplot[mark=*, fill=red!70!white] coordinates {(0,0.72) (1,0.72)};
		\addplot[mark=*, fill=red!70!white!30!] coordinates {(0,0.42) (1,0.42)};
	\end{axis}
\end{tikzpicture}
\end{document}

You can see how red!70!white which is 70% red and 30% white, is different from the red!70!white!30! that this code generates.

Maybe it is more clear if we have the from_mix method to output the format:
{rgb:<Color_1>,<weight_1>;...;<Color_N>,<weight_N>}
which surprisingly works with any color combination, not just Red, Green, and Blue. We could make it even work with already mixed colors.

Something like:

fn from_mix<I, C>(colors: I) -> Self
    where 
        C: Into<Self>,
        I: impl IntoIterator<Item=(C, u8)>
{
    let mut result = String::new();

    result.push_str("rbg:");

    for (color, num) in vec {
        let color = Color::from(color);

        result.push_str(&color.to_string());
        result.push(',');
        result.push_str(&num.to_string());
        result.push(';');
    }

    result.pop(); // Remove the trailing semicolon

    result
}

Using the above with vec![(Red, 70), (White, 30)] produces the correct color.

It also has the benefit that it is easier to document the u8s as "weights" rather than "percentages" (given that we won't be checking user input to add to 100%)

[tomgroenwoldt]

Done :)

[DJDuque]

Note to self:
Should this be #[strum(disabled)] given that you are handling this case explicitly in the Marker's Display?

[tomgroenwoldt]

True!