nulinspiratie / SilQ

Software for quantum control of donor atoms in silicon
Other
6 stars 1 forks source link

Matplotlib kwarg aliases are used inconsistently and might cause errors in future #206

Closed maij closed 3 years ago

maij commented 4 years ago

Basically, a newer version of maplotlib has deprecated the following usage and will instead raise a TypeError:

x = np.linspace(0, 1, 10)
y = np.exp(x)
plt.plot(x, y, c='r', color='blue') # 'c' aliases to 'color' and is defined twice, this will raise an error

This might seem silly, but this can occur when we have plotting wrappers, e.g. in the in the fit toolbox.

Where the default kwargs are: plot_kwargs = {'linestyle': '--', 'color': 'cyan', 'lw': 3}

 def add_to_plot(self,
                    ax: Axis,
                    N: int = 201,
                    xrange: Tuple[float] = None,
                    xscale: float = 1,
                    yscale: float = 1,
                    **kwargs):
        """Add fit to existing plot axis

            kwargs: Additional plot kwargs. By default Fit.plot_kwargs are used

and a bit later...

        """
        plot_kwargs = {**self.plot_kwargs, **kwargs} # This can cause issues
        self.plot_handle, = ax.plot(
            x_vals_full, y_vals_full, **plot_kwargs
        )
        return self.plot_handle

Now I'm not sure how widespread this issue is, but there are tools available within matplotlib to clean up kwargs such as:

>>> from matplotlib import cbook
>>> plot_kwargs = {'linestyle': '--', 'color': 'cyan', 'lw': 3}
>>> cbook.normalize_kwargs(plot_kwargs, alias_mapping=dict(linewidth=['lw']))
{'linewidth': 3, 'linestyle': '--', 'color': 'cyan'}

I would suggest at some point we look through how we handle plotting throughout silq and try to centralize it with rcParams or something.

maij commented 3 years ago

Update:

I thought I found an elegant solution in creating an rc_context, that way you can't accidentally doubly-define any kwargs. Something like the following:

rc_fit_style = {
   'lines.linestyle': '--',
   'lines.linewidth': 2,
   'lines.color': 'k' #problem here...
}

...

with plt.rc_context(rc_fit_style):
   ax.add_to_plot(x,y, **kwargs) # where kwargs contains any user arguments 

The problem with this approach is two-fold. 'lines.color' is not a proper rcParam, so it simply doesn't work. Furthermore, the only way that I know to get around this is to change the cycler object for the axes, i.e. the colours that plt.plot cycles through when adding multiple lines. Unfortunately, simply using rcParams you can't change the cycler object for any pre-existing axes, which is quite common when you're adding a fit to already plotted data. (for more info see here.

So you're stuck having some arguments in an rc_context (linestyle, linewidth etc), while some other arguments i.e. the line color, have to be passed through as a kwarg which is messy.

This tiny problem is surprisingly fraught with pitfalls!

nulinspiratie commented 3 years ago

Yes it sounds like a surprisingly annoying problem. I'm afraid I can't help you much here, I don't have much experience with this...