Skip to content

Classmethod flows lose class binding after .with_options() #17354

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.

Sign up for GitHub

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

Jump to bottom
Closed
bnaul opened this issue Mar 4, 2025 · 1 comment · Fixed by #17647
Closed

Classmethod flows lose class binding after .with_options() #17354

bnaul opened this issue Mar 4, 2025 · 1 comment · Fixed by #17647
Labels
bug Something isn't working

Comments

@bnaul
Copy link
Contributor

bnaul commented Mar 4, 2025 •
edited
Loading

Bug summary

On Prefect 3.2.9, classmethod flows lose their cls inheritance structure when copied with .with_options():

Minimal reproducer:

from prefect import flow

class BaseProcessor:
    @classmethod
    def get_multiplier(cls):
        return 1
    
    @classmethod
    @flow
    def process(cls, x: int):
        return x * cls.get_multiplier()

class ChildProcessor(BaseProcessor):
    @classmethod
    def get_multiplier(cls):
        return 2

# Original flow works correctly
assert BaseProcessor.process(5) == 5    # Returns 5
assert ChildProcessor.process(5) == 10  # Returns 10

# After with_options(), cls binding is lost and inheritance breaks
new_flow = ChildProcessor.process.with_options()
assert new_flow(5) == 10  # This fails - loses class context

I would definitely expect the original cls reference to be preserved by the copied flow.

Version info

Version:             3.2.9
API version:         0.8.4
Python version:      3.10.16
Git commit:          27eb408c
Built:               Fri, Feb 28, 2025 8:12 PM
OS/Arch:             darwin/arm64
Profile:             default
Server type:         cloud
Pydantic version:    2.10.6
Integrations:
  prefect-gcp:       0.6.2
  prefect-dask:      0.3.3
  prefect-kubernetes: 0.5.3

Additional context

No response
@bnaul bnaul added the bug Something isn't working label Mar 4, 2025
@cicdw
Copy link
Member

cicdw commented Mar 4, 2025

There's something weird and stateful going on here related to the descriptor protocol implemented here.

Here's an example where your test passes:

# Original flow works correctly
assert BaseProcessor.process(5) == 5    # Returns 5
assert ChildProcessor.process(5) == 10  # Returns 10

# After with_options(), cls binding is lost and inheritance breaks
new_flow = ChildProcessor.process.with_options()

## call as a classmethod first
assert ChildProcessor.process(5) == 10  # Returns 10
assert new_flow(5) == 10  # This now succeeds

vilhenad added a commit to vilhenad/prefect that referenced this issue Mar 26, 2025
@vilhenad
Fix PrefectHQ#17354: Preserve class binding in classmethod flows when…
8b9f61b 
… using .with_options()

Classmethod flows were losing their cls inheritance structure when copied with .with_options(), causing them to default to the base class implementation. This was particularly evident when subclassing (e.g., ChildProcessor.process would incorrectly use BaseProcessor.get_multiplier).

The fix ensures proper context preservation by:
1. Creating deep copies of the function for each bound flow
2. Maintaining independent __prefect_self__ references
3. Preserving all function attributes during copying

Added test cases verify:
- Classmethod inheritance is maintained after .with_options()
- Instance methods retain their proper self context
- Multiple bound flows don't interfere with each other
vilhenad added a commit to vilhenad/prefect that referenced this issue Mar 26, 2025
@vilhenad
Fix PrefectHQ#17354: Preserve class binding in classmethod flows when…
3fb9a3f 
… using .with_options()

Classmethod flows were losing their cls inheritance structure when copied with .with_options(), causing them to default to the base class implementation. This was particularly evident when subclassing (e.g., ChildProcessor.process would incorrectly use BaseProcessor.get_multiplier).

The fix ensures proper context preservation by:
1. Creating deep copies of the function for each bound flow
2. Maintaining independent __prefect_self__ references
3. Preserving all function attributes during copying

Added test cases verify:
- Classmethod inheritance is maintained after .with_options()
- Instance methods retain their proper self context
- Multiple bound flows don't interfere with each other
vilhenad added a commit to vilhenad/prefect that referenced this issue Mar 26, 2025
@vilhenad
Fix PrefectHQ#17354: Preserve class binding in classmethod flows when…
19d765a 
… using

.with_options()

Classmethod flows were losing their cls inheritance structure when
copied with .with_options(), causing them to default to the base class
implementation. This was particularly evident when subclassing
(e.g., ChildProcessor.process would incorrectly use
BaseProcessor.get_multiplier).

The fix ensures proper context preservation by:
1. Creating deep copies of the function for each bound flow
2. Maintaining independent __prefect_self__ references
3. Preserving all function attributes during copying

Added test cases verify:
- Classmethod inheritance is maintained after .with_options()
- Instance methods retain their proper self context
- Multiple bound flows don't interfere with each other
@vilhenad vilhenad mentioned this issue Mar 30, 2025
Merged
4 tasks
vilhenad added a commit to vilhenad/prefect that referenced this issue Apr 7, 2025
@vilhenad
Fix PrefectHQ#17354: Preserve class binding in classmethod flows when…
aeeb09a 
… using

.with_options()

Classmethod flows were losing their cls inheritance structure when
copied with .with_options(), causing them to default to the base class
implementation.

The fix ensures proper context preservation by:
1. Creating deep copies of the function for each bound flow
2. Preserving all function attributes during copying

Added test cases verify:
- Classmethod inheritance is maintained after .with_options()

Closes PrefectHQ#17354
vilhenad added a commit to vilhenad/prefect that referenced this issue Apr 7, 2025
@vilhenad
Fix PrefectHQ#17354: explicitly copy binding attributes (__prefect_cl…
518f246 
…s__ and __prefect_self__) so that instance methods don't lose context and don't fail parameter binding.
vilhenad added a commit to vilhenad/prefect that referenced this issue Apr 30, 2025
@vilhenad
Fix PrefectHQ#17354: Preserve class binding in classmethod flows when…
33e7421 
… using

.with_options()

Classmethod flows were losing their cls inheritance structure when
copied with .with_options(), causing them to default to the base class
implementation.

The fix ensures proper context preservation by:
1. Creating deep copies of the function for each bound flow
2. Preserving all function attributes during copying

Added test cases verify:
- Classmethod inheritance is maintained after .with_options()

Closes PrefectHQ#17354
vilhenad added a commit to vilhenad/prefect that referenced this issue Apr 30, 2025
@vilhenad
Fix PrefectHQ#17354: explicitly copy binding attributes (__prefect_cl…
073dea8 
…s__ and __prefect_self__) so that instance methods don't lose context and don't fail parameter binding.
vilhenad added a commit to vilhenad/prefect that referenced this issue Apr 30, 2025
@vilhenad
Fix PrefectHQ#17354: switched @classmethod decorator with @flow decor…
2454587 
…ator to ensure compatability with python 3.13.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add test covering .with_options when used with a classmethod flow vilhenad/prefect
2 participants
@bnaul @cicdw