Skip to content

Conversation

@PatKamin
Copy link
Contributor

No description provided.

@PatKamin PatKamin requested a review from a team as a code owner October 24, 2025 11:14
@PatKamin
Copy link
Contributor Author

@lslusarczyk, @lukaszstolarczuk, Is the compute.py refactor commit the way we should go? I think it makes moving through this code easier. If yes, I will follow up with all the other benchmarks' modules.

Improve readability:
- group methods and attributes into public, protected (underscore prefix)
  and private (double underscore prefix)
- order public methods to align with their order in the base
  Benchmark class

def description(self) -> str:
return ""

Copy link
Contributor

Choose a reason for hiding this comment

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

in general I like this cleanup, but I guess we may quickly forget about this "new order" - perhaps add "sections", e.g. put a comment here. Just an idea - please do as you think.

#
## Protected functions
#


def description(self) -> str:
return ""

Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps also, describe this shortly in our contrib guide...?

## Adding New Benchmarks

1. **Create Benchmark Class:** Implement a new class inheriting from `benches.base.Benchmark`. Implement required methods (`setup`, `run`, `teardown`, `name`) and optional ones (`description`, `get_tags`, etc.) as needed.
1. **Create Benchmark Class:** Implement a new class inheriting from `benches.base.Benchmark`. Implement required methods (`run`, `name`) and optional ones (`description`, `get_tags`, etc.) as needed.
Copy link
Contributor

Choose a reason for hiding this comment

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

setup() is mentioned above as required method

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