Skip to content

Conversation

@Nomos11
Copy link
Collaborator

@Nomos11 Nomos11 commented Jul 29, 2025

No description provided.

@Nomos11 Nomos11 requested a review from shumpohl July 29, 2025 09:53
@github-actions
Copy link

github-actions bot commented Jul 29, 2025

Test Results

    6 files      6 suites   7m 19s ⏱️
1 223 tests 1 161 ✅  62 💤 0 ❌
7 338 runs  6 966 ✅ 372 💤 0 ❌

Results for commit f7c3c2b.

♻️ This comment has been updated with latest results.



@property
def contains_sweepval(self) -> bool:
Copy link
Member

Choose a reason for hiding this comment

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

Would it be a great churn to rename this to contains_dynamic_value and make it a method instead of a property?

Copy link
Collaborator Author

@Nomos11 Nomos11 Aug 1, 2025

Choose a reason for hiding this comment

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

should be no problem.

what is the benefit here of a method over property?

Copy link
Member

@shumpohl shumpohl left a comment

Choose a reason for hiding this comment

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

Discuss replacement of sweepval with dynamic_value

@shumpohl shumpohl merged commit 1798978 into master Aug 14, 2025
9 checks passed
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.

3 participants