Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception when creating copy of Option object with Constraint #166

Closed
MePsyDuck opened this issue Aug 21, 2023 · 5 comments · Fixed by #167
Closed

Exception when creating copy of Option object with Constraint #166

MePsyDuck opened this issue Aug 21, 2023 · 5 comments · Fixed by #167
Labels
bug Something isn't working
Milestone

Comments

@MePsyDuck
Copy link

MePsyDuck commented Aug 21, 2023

Using:

  • python 3.10
  • click 8.1.7
  • cloup 3.0.0

Bug description

Any class that extends cloup.constraints.Constraint cannot be copied via copy package or copy() method.

To Reproduce

Here's a minimal example to reproduce the issue

import copy
from cloup.constraints import Constraint

class MyConstraint(Constraint):
    def help(self, ctx):
        return 'help text'

    def check_values(self, params, ctx):
        pass

print(getattr(MyConstraint(), "__reduce_ex__", None)) # return non-None value
copy.copy(MyConstraint()) # produces error
MyConstraint().copy() # produces error

Output

Traceback (most recent call last):
  File ".../test.py", line 12, in test
    copy.copy(MyConstraint())
  File ".../python3.10/copy.py", line 92, in copy
    rv = reductor(4)
TypeError: 'NoneType' object is not callable

A library that I use (click-extra) uses deepmerge to build a parameter tree. When attempting to make a deepcopy of the Option object, a copy of the constraint is necessary, but instead it results in the exception mentioned above.

Since my understanding of Python is limited, I'm unable to debug the issue on my own. However, I have managed to narrow down the problem to this point. If there's a way I can assist in resolving this issue, please let me know.

@MePsyDuck MePsyDuck added the bug Something isn't working label Aug 21, 2023
@janluke
Copy link
Owner

janluke commented Aug 21, 2023

Thanks for reporting. I'm quite busy these days but I'll see what I can do this week. Otherwise, @kdeldycke feel free to have a look at it :)

@janluke
Copy link
Owner

janluke commented Aug 23, 2023

Fix released in v3.0.1.

@janluke janluke added this to the v3.0.1 milestone Aug 23, 2023
@kdeldycke
Copy link
Contributor

Oh thank both of you for this! Thank you @MePsyDuck for taking the time to dig in into Click Extra internals, and post an issue here, even if you don't know Python. 🤗

I was not aware of this issue and used deepmerge a little bit blindly, not knowing its detailed implementation. But now, in light of @janluke fix I get it.

Huge respect for you @janluke for looking into it and cutting a release while being busy. 💪 Also busy myself so I'll make the effort to release a new bug-fix version of Click Extra too. 😅

kdeldycke added a commit to kdeldycke/click-extra that referenced this issue Aug 23, 2023
@MePsyDuck
Copy link
Author

Great to see the issue resolved in version 3.0.1! Thanks, @janluke , for the quick fix despite your busy schedule.
Also, thank you, @kdeldycke , for your kind words! I'm glad I could contribute. Looking forward to continued improvements.

@janluke
Copy link
Owner

janluke commented Aug 23, 2023

But now, in light of @janluke fix I get it.

I actually don't have a full understanding of it :D. I replicated the issue and read an error message that didn't make any sense:
image

I just thought it might be related to __getattr__ and noticed the mistake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants