Add argument names to enum_ methods

[YannickJadoul]

Description

Add generic names to the methods added by enum_, to avoid autogenerated arg0, etc. names.

This is not a full solution to #2623, but I think it's the best we can easily do? Taking extra parameters seems reasonably over the top, and I can't immediately come up with a better solution. Suggestions welcome, though.

Closes #2623

Suggested changelog entry:

Added parameter names to the ``py::enum_`` constructor and methods, avoiding ``arg0`` in the generated docstrings.

[YannickJadoul]

@andioz, if you would want to have a look, and double-check if I didn't miss anything, please go ahead! :-)

[andioz]

@YannickJadoul, thanks for the quick implementation! I tried the new code, it looks good. If I understand it right, this pull request changes the hard-coded argument names from arg0 to value for enums, to state for __setstate__ methods, respectively 3 more argument names to other (I cannot find out for which methods right now).

You say this is no full solution, you mean there is still no method to customize the name, right? Well thats fine right now, good to have good default names.

I would suggest to add all changed names to the changelog, and maybe mention it in the documentation?

And finally, I would suggest to add piece of code to the tests, just to show the changed behavior.

Good job 👍

[YannickJadoul]

Thanks, @andioz!

respectively 3 more argument names to other (I cannot find out for which methods right now).

Those are the macros for all the comparison operators (__eq__, __lt__, etc).

You say this is no full solution, you mean there is still no method to customize the name, right? Well thats fine right now, good to have good default names.

Yes, that's what I meant. After all, I agree arg0 looks rather ugly, but value should work for most people in 99% of the cases? And if not, there are workarounds anyway (i.e., manually turning a class_ into an enumeration type).

I would suggest to add all changed names to the changelog, and maybe mention it in the documentation?

Is it that big/important? The changelog (see above) will point here anyway, and for the docs I'm not convinced this isn't just going to be "noise" (users would just notice in their docstring signatures; I'm not sure there's anything to explain about how this works?).

[andioz]

Regarding documentation, I agree, it is not important enough. But a changelog entry should be there. And a short test ;) I've seen there are test for docstrings available.

[YannickJadoul]

Yep, the changelog gets added afterwards; it's in the original message at the top of this PR (it gets added afterwards, to avoid too many merge conflicts).

Tests could be possible. I also don't think it's that urgent for this kind of case, but it can't really hurt either, I suppose :-)

[andioz]

OMG - you should write tests before changing code 😆 Well, just kidding, but seriously, I learned to find it really useful to write a test before changing code. Anyway, your choice.

[YannickJadoul]

Hahaha, myeah, I know. But I'm too focused on fixing it, and especially in this case, it feels so immediate that ... yeah. When it gets complex, I do sometimes stash my own changes to see if tests failed before, though ;-)

[andioz]

I'm fine with it :) So, do you think about closing the issue at all, even functions for customization are not available yet? With some rejection note?

[YannickJadoul]

So, do you think about closing the issue at all, even functions for customization are not available yet? With some rejection note?

Well, yeah. It's kind of solved, no? I thought this was at least good enough, unless someone can come up with some great idea. Or don't you agree?

[andioz]

Agreed and approved 👍 I close the issue, thanks Yannick

[YannickJadoul]

Agreed and approved +1 I close the issue, thanks Yannick

No worries. This will automatically close that issue ;-)

[andioz]

haha I'm not used to this github stuff, but I will learn

[YannickJadoul]

Given this "Closes #2623" (or "Fixes") in the main description above, merging this PR will close that issue automatically.

[andioz]

Oh a test! Great!! 👏

[YannickJadoul]

Not sure what else to test, so I'm going to call this done ;-)

[andioz]

Sure, good test are often pretty simple, if the code base is clean. Thanks for all, great work!

[rwgk]

This PR passed Google-global testing.
I strongly support merging now, so it ships with 2.6.1.

[henryiii]

That's good enough for me.