Document comparison

[hynek]

These are some narrative docs for #627, I'm open to suggestions to make it juicier.

Here's the built version: https://attrs--768.org.readthedocs.build/en/768/comparison.html

@Julian @botant

[botant]

Looks good to me, simple and direct.

1. There is a typo in the first sentence:

By default, two instances of attrs classes is equal, if all their fields are equal. For that, attrs writes eq and ne methods for you.

2. I'd add a case insensitive comparison, since this was one of the requests.

3. I'd mention pandas DataFrames as well as numpy arrays. Is there a way to provide an example without having to import those libraries?

4. I'd make it clear that specifying a callable for eq does not mean the same key will be used for order; the key must be specified explicitly.

[hynek]

I believe I have addressed everything except 3. I'd be happy to add a fake doctest but someone else has to write it for me because I've never used neither NumPy nor Pandas. 😳

[botant]

I just realized that we were going to provide a wrapping class, since numpy and pandas do not follow the key model. In both cases, you need to use a comparison function that takes 2 arguments. This was discussed in the original PR.

Shall we provide a function like attrs.cmp.using(key: Callable) -> Callable? The output callable is a function that returns a wrapper object that uses key as eq. Or something like that.

Alternatively, and probably easier, we can not mention numpy in this release, and add it back via a different PR.

[hynek]

Hm, I think it would be nicer to publish all at once if it isn't too much work. We had decided on the API IIRC and it shouldn't be too complex to add? 🤔 Just adding the APIs that landed without NumPy support seems to be lacking the main effect? :)

[botant]

That works as well. I'll work on it later today, if that is okay?

[botant]

There is another point on cmp, and that might be what you meant to ask in the other PR.

Should we de-deprecate it and keep it permanently as syntactic sugar? That would save users from having to wirte attr.ib(eq=int, order=int) for a nicer looking attr.ib(cmp=int).

Just a thought.

[hynek]

That works as well. I'll work on it later today, if that is okay?

It's not like I can cut your salary. ;) If we get it done by tomorrow it's be more than great. RedHat wants a release but there's some minor stuff I need to address first anyways.

Should we de-deprecate it and keep it permanently as syntactic sugar?

Yes we should. It was very obvious to me while writing the docs. :)

[botant]

How about something along these lines? The same logic used to validate cmp, eq and order in attr.ib() would be used here to regulate the usage of the arguments.

attr.cmp.using(eq: Callable = None, order: Callable = None, cmp: Callable = None, enforce_same_class: bool = True) -> Callable

def using(eq: Callable = None, order: Callable = None, cmp: Callable = None, enforce_same_class: bool = True) -> Callable:

    @attr.s(eq=False, order=False)
    class Comparable(object):
        value = attr.ib()
        def __eq__(self, other):
            pass
        def __lt__(self, other):
            pass
        ...

    return Comparable

Note: I think we should let the user choose whether equal classes should be enforced, since the whole point of the PR is to let the user customize the operation.

[hynek]

If everything is done by one function, what's the point of the module? Storing helpers?

[botant]

To follow the pattern of validators, etc, but most of all I thought it looked good.

[hynek]

I'm not opposed to it, I'd just like to think it through. I could live with a helper for Numpy/Pandas for instance, if it's possible without importing them.

[botant]

I wouldn't add specific helpers for numpy/pandas. Too much trouble and too little added value, if all the user needs is using(key=np.array_equal) or something similar. Plus, I'm afraid that one particular helper might not cover all use cases related to matrices, time series, etc. I think one simple function is the way to go. Maybe attr.cmp_using?

[Julian]

Cool!

[hynek]

Maybe attr.cmp_using?

Oooof sorry I lost the thread on this one! Yes please, I’d like to push 21.1.0 but realized this is still open. Any chance to get it done sometime next week?

[botant]

Working on this now.