Skip to content

Add Sketch.is_looping as property - #804

Draft
ulgens wants to merge 1 commit into
py5coding:mainfrom
ulgens:789-is_looping
Draft

ulgens wants to merge 1 commit into
py5coding:mainfrom
ulgens:789-is_looping

Conversation

@ulgens

@ulgens ulgens commented Sep 16, 2026

Copy link
Copy Markdown

Will be updated after manual testing.

Adds Sketch.is_looping property and resolves #789 .

fget=_get_is_running, doc="""$class_Sketch_is_running"""
)

@property

@ulgens ulgens Sep 16, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

is_ready and is_running handles similar implementations by creating a helper method, then wrapping it as a property. I checked the codebase but couldn't understand the benefit of this approach, it seems to complicate the code for no reason.

Property-wrapping approach causes a typing issue too:

Image

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I checked the codebase but couldn't understand the benefit of this approach, it seems to complicate the code for no reason.

py5generator is essentially a custom template engine that takes code templates and adds more code to them to make complete python files. The template engine needs to be able to read and parse the code in the templates and extract information from them to maintain the type signatures and compare them with the documentation files. The code that extracts information expects the code to be written in a certain way to be easily parsed.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Any chance to give some context about why template engine needs it? I couldn't see how _get... + property wrapper approach provides anything extra over the approach in this PR.

mock_methods = MockMethods(s)
s.loop = mock_methods.mock_loop
s.no_loop = mock_methods.mock_no_loop
s.is_looping = mock_methods.mock_is_looping

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Not sure this is actually needed and how to handle the property. Will revisit.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not sure this is actually needed and how to handle the property. Will revisit.

Good catch. I think mocking this would be important for is_looping to work properly with live coding.

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.

Adding .is_looping

2 participants