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

Fix depending on the method of a sub parameter object #765

Merged
merged 2 commits into from
Jun 19, 2023

Conversation

maximlt
Copy link
Member

@maximlt maximlt commented Jun 19, 2023

Fixes #764

This PR fixes a regression that was introduced in #753. It appeared that depending on a method was pretty much untested so I have added some more tests.

When _spec_to_obj resolves a dependency on a Parameterized method, the type of the resolved method is:

  • FunctionType if it's a direct dependency, as in @param.depends('method'). The object holding the reference to the method seen by _spec_to_obj is a class.
  • MethodType if it's a dependency on a sub object, as in @param.depends('sub.method'). The object holding the reference to the method seen by _spec_to_obj is an instance.

Good to know!

from types import FunctionType, MethodType

class A:
    def foo(self): pass

assert isinstance(A.foo, FunctionType)
assert isinstance(A().foo, MethodType)

The change made in this PR is pretty simple, I'll however kindly ask @philippjfr to review it to ensure that this is correct, and check that the problem actually doesn't lie with how the object holding the method reference is resolved (class vs. instance).

@maximlt maximlt requested a review from philippjfr June 19, 2023 12:36
Copy link
Member

@philippjfr philippjfr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix! The change makes sense to me, I think this wasn't originally implemented because the actual decorator is given the function before it is turned into a method.

@maximlt maximlt merged commit 8759c1c into main Jun 19, 2023
@maximlt maximlt deleted the fix_method_subparam branch June 19, 2023 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Depending on the method of a sub parameter object is broken
2 participants