Reprs on formatters and locaters

@StefRe

As discussed in #21874, there aren't reprs on the locators and formatters. Reprs of the form where eval(repr) = call, something like

eval('AutoDateLocator(maxticks=8)') = AutoDateLocator(maxticks)

would mean reprs could be used in the documentation examples, which would help keep the labels in sync. This is useful for the new example #21874 &

https://matplotlib.org/devdocs/gallery/ticks/tick-formatters.html
https://matplotlib.org/devdocs/gallery/ticks/tick-locators.html
and possibly reproducibility

I think it's a good first issue 'cause it's fairly standalone and rote if you know classes, only potential difficulty might be in storing the original args:

When I try AutoDateLocator.__repr__ = lambda self: f'AutodateLocator(maxticks={self.maxticks})' then repr(AutoDateLocator(maxticks=8)) returns 'AutodateLocator(maxticks={0: 8, 1: 8, 3: 8, 4: 8, 5: 8, 6: 8, 7: 8})', i.e. the class variable maxticks. I don't know how to get the value passed in during the function call. But even if this is possible, I find defining all the repr rather verbose and a bit hacky as compared to a simple eval.

Originally posted by @StefRe in #21874 (comment)

only potential difficulty might be in storing the original args:

I don't see how you are going to do that without adding a bunch of verbiage to the init, which I wouldn't be in favour of just to make one example more pythonic.

@anntzer

I don't see how you are going to do that without adding a bunch of verbiage to the init

Kinda hoping @anntzer will tell me there's some sorta magic you can stick on the base class that will just handle it, which really yeah I dunno that I'd be in favor of reprs if they were overly complicated, but might be worth someone poking and seeing if it can be done cleanly.

It wouldn't just be to make the example more pythonic - the repl can be useful for debugging & reproducibility - if a formatter is being weird, it'd be great to do a quick first pass by printing - especially the default formatters.

I'm fine with a clean repr that can be based on the current state of the Locator or Formatter. I'm just leery of magic to capture what the user typed as arguments.

Something like

import functools
import inspect


def with_capture(init):
    @functools.wraps(init)
    def wrapper(self, *args, **kwargs):
        call = (
            str(inspect.signature(wrapper.__get__(self)).bind(*args, **kwargs)).
            removeprefix("<BoundArguments ").removesuffix(">"))
        self._repr = f"{type(self).__name__}{call}"
    return wrapper


class ReprFromInit:
    def __init_subclass__(cls):
        cls.__init__ = with_capture(cls.__init__)

    def __repr__(self):
        return self._repr


class T(ReprFromInit):
    def __init__(self, foo, bar):
        pass


print(repr(T(1, 2)))
print(repr(T(1, bar=2)))

? No guarantees, may brick your homework, eat your dog, or whatnot.

@story645

@story645 Heyyo, so I've got a question about this. Should we include RadialLocator, ThetaLocator. and ThetaFormatter from projections/polar.py? They aren't mentioned in the docs (https://matplotlib.org/devdocs/api/ticker_api.html, https://matplotlib.org/devdocs/gallery/ticks/tick-formatters.html, https://matplotlib.org/devdocs/gallery/ticks/tick-locators.html), but they do decorate the Locator/ Formater classes so they still technically inherit from them.

The reason why it's a problem is because it conflicts with my first assumption in my PR, specifically RadialLocator which you can pass in an Axis object that doesn't have a working repr at init time like PolarAxes.

Um so I think they should definitely be documented, possibly in their own example like datetime

As for the technical question, is the working repr a must or something that can be special cased?

@jklymak

I do also wish to reassert @jklymak's comment from above:

I'm fine with a clean repr that can be based on the current state of the Locator or Formatter. I'm just leery of magic to capture what the user typed as arguments.

Both the code above and that of the PR only capture the starting state, and so will be misleading if the values are set after instantiation.

:/ but I do see your point that it should reflect current state if it's gonna be useful for debugging, even if'd rather it reflect initial state for the purpose of the examples.

As for the technical question, is the working repr a must or something that can be special cased?

Just to clarify, based on my testing, the repr works, it's just that when we are initing a new PolarAxis object it's missing something for the repr to work. I think I can fix it two ways:

Since from the unit tests it looks like PolarAxes objects are the only ones being passed like this, I could treat it like a special case like you suggest.
Figure out exactly what's missing for the repr to work and fiddle with how it's being inited, so it works by the time we call it.

Regarding reflecting the current state, I think this is possible if we just got all the param names from the init function in init_subclass like anntzer's example, and grabbed the names of all kwargs, from a wrapped init.

Then at run time in the repr call, we simply get all the values using getattr. This would work as long as the names of the args matched the names of the attributes exactly (a small difference like a _ in the front could be accounted for though). For any attributes that don't match, we can just manually add mappings in a private class variable to avoid making any api changes. Killing two birds with one stone, doing it like this will most likely fix the issue with RadialLocator too.

@story645

@story645 Does the position of each arg matter? I have a (rough) working solution currently like:

class Parent:
    def __repr__(self):
        if (not hasattr(self, "_reprPropNames")):
            return self.__class__.__name__
        propvalues = [self._get_prop_value(name) for name in self._reprPropNames]
        return f"{self.__class__.__name__}({Parent.printKWArgs(zip(self._reprPropNames, propvalues))})"
    
    @staticmethod
    def printKWArgs(kwargs):
        return ", ".join([f"{key}={val.__repr__()}" for key,val in kwargs])
    
    def _get_prop_value(self, propName):
        try:
            return getattr(self, propName)
        except AttributeError:
            pass
        
        try:
            return getattr(self, '_' + propName)
        except AttributeError:
            pass
        return ""

    def __init_subclass__(child):
        if (child.__init__ and hasattr(child.__init__, "__wrapped__")):
            originalInit = child.__init__.__wrapped__
        else:
            originalInit = child.__init__
            
        signature = inspect.signature(originalInit).__str__().removeprefix('(').removesuffix(')')
        arg_names = [arg.split("=")[0].removeprefix(" ") for arg in signature.split(",") if '*' not in arg][1:]

        requires_mapping = hasattr(child, "_arg_attribute_map")

        if (requires_mapping):
            arg_names = [(child._arg_attribute_map[arg] if child._arg_attribute_map.get(arg) else arg) for arg in arg_names]

        def _filter_kwargs(kwargs):
            if (requires_mapping):
                old_mappings = list(child._arg_attribute_map.keys())
                return list(set(old_mappings) - set(kwargs)) + list(set(kwargs) - set(old_mappings))
            return kwargs

        @wraps(originalInit)
        def _init_repr_wrapper(self, *args, **kwargs):
            _init_repr(self, list(kwargs.keys()))
            originalInit(self, *args, **kwargs)

        def _init_repr(self, kwargNames):
            if (not hasattr(self, "_repr")):
                self._reprPropNames = set(arg_names + _filter_kwargs(kwargNames))

        child.__init__ = _init_repr_wrapper

but it does not guarantee that the args will match the order of the init method. For ex:

class Child(Parent):
    _arg_attribute_map = {"invalidMap": "validMap"}

    def __init__(self, bool=False, invalidMap='foo', *, foo="bar", **kwargs):
        self._bool = bool
        self.foo = foo
        self.kwargsTest = kwargs['kwargsTest']
        self.validMap = invalidMap

child = Child(True, foo="test1", invalidMap='test2', kwargsTest="test3")
print(child.__repr__())

prints

Child(validMap='test2', kwargsTest='test3', foo='test1', bool=True)

Um mostly depends on how much extra code would be needed to make things match, but I don't consider it a blocker.

I'm leaning towards anything that tries to be magical and work for all subclasses is likely to be fragile at best... (And a confusing not very well trodden path to figure out what is going wrong at worst).

I would tend to agree with #25767 (comment) where the solution is less of a "try to write something general" and more of a "go through each of the first party classes and write simple reprs for each"

The fact of the matter is the reason such a magical repr doesn't already exist is that there is no requirement that the inputs to __init__ are stored directly in variables that are so trivial to pick out, nor especially that the internal state has names that are related to the __init__ parameters.

The example given above is incorrect because the __init__ arg is named invalidMap but the internal state is called validMap, and that is what is put in the repr. You could try to disambiguate on value, but that will quickly run into problems if multiple fields have the same value.

In short, I don't think it can be done without imposing restrictions on subclasses (such as naming of internal state). The alternative is to write bespoke reprs for each class, which is not as interesting of a problem, but elides nearly every concern by putting implementation local to the individual classes which know what their parameters are and how to back calculate them. The only additional concern is therefore that subclasses further down the class hierarchy may get incorrect reprs inherited. This is solved by implenting reprs for that specifically, but that must be done by any downstream library that makes one. We can at least use type(self).__name__ to get the class name correct, and if params/backcalculations are unchanged that will work.

@Daniil-Aleshechkin

I don't have a strong bias in either direction - I like a catch all approach b/c it catches everything in one shot and is probably more fun to code, but I agree that individual reprs in each class will probably be easier to maintain. And very possibly altogether add less verbiage than the automatic solutions, which is how we got down that rabbit hole in the first place - @Daniil-Aleshechkin how do you feel about going simple?

@ksunden

Yea actually I agree with @ksunden, making this restriction will basically couple the existing names or values to the repr, and it's not exactly clear from each class without having to do some deep diving how the repr method is generated. Plus I'd admit this solution isn't exactly simple or pythonic. Also it's not guaranteed to grab everything. What I'm leaning towards is to have a general repr, but it's dependent on some function like _get_repr_params which returns a dictionary of all params we'd care about for each class. Something like:

class Parent:
    def _get_repr_params(self):
        return {}

    def __repr__(self):
        return f"{self.__class__.__name__}({Parent.printKWArgs(_get_repr_params())})"
    
    @staticmethod
    def printKWArgs(kwargs):
        return ", ".join([f"{key}={val.__repr__()}" for key,val in kwargs])

class Child(Parent):
    def _get_repr_params(self):
        return {"bool" : self.bool, "foo" : self.foo}

    def __init__(self, bool=False, invalidMap='foo', *, foo="bar", **kwargs):
        self._bool = bool
        self.foo = foo
        self.kwargsTest = kwargs['kwargsTest']
        self.validMap = invalidMap

What do you think?

One problem about it is we can't enforce an eval(repr()) = call relationship. Though we can always just test that with unit tests, and that will be good enough for maintainability.

One problem about it is we can't enforce an eval(repr()) = call relationship.

Many of these classes have state that can only be set after the fact so ensuring that this always holds is not currently possible (that is if the repr is the init call than there can be two objects that are functionally different (swapping them on an Axis changes the perceived visual appearance) but have identical reprs).

We have a number of ad-hoc protocols like this in the codebase already, however they need to save a fair amount of complexity to be worth it. In this case it is not clear to me that defining a special private method to source a dictionary to to pass to an over-ridable function static method from the parent class is simpler or easier to explain when compared to

    def __repr__(self):
        return f'{self.__class__.__name__}(a={self._a}, b={self.b})'

in each class!

I think the general solution would eventually converge to re-writing attrs or dataclasses piece by piece and that is a much larger project (and if we went that route we probably should just use one of the "managed attribute" libraries that are out there like traitlets, attrs, pydandtic, dataclasses, etc rather than roll our own).

we can't enforce an eval(repr()) = call

So while I put this in as a motivation, this isn't a blocker by any means since I primarily want reprs for the debugging and the examples can be tweaked through other means.

Add __repr__ to Formatter and Locator classes (fixes matplotlib#21898)

Navigation Menu

Repository navigation

Reprs on formatters and locaters #21898

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Reprs on formatters and locaters #21898

Description

Activity

jklymak commented on Dec 10, 2021

story645 commented on Dec 10, 2021

jklymak commented on Dec 10, 2021

anntzer commented on Dec 10, 2021

Daniil-Aleshechkin commented on May 1, 2023

story645 commented on May 1, 2023

ksunden commented on May 1, 2023

story645 commented on May 1, 2023

Daniil-Aleshechkin commented on May 1, 2023

Daniil-Aleshechkin commented on May 3, 2023

story645 commented on May 3, 2023

ksunden commented on May 4, 2023

story645 commented on May 4, 2023

Daniil-Aleshechkin commented on May 4, 2023

tacaswell commented on May 4, 2023

story645 commented on May 4, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions