search

dyson-ai / bencher

A package for benchmarking the characteristics of arbitrary functions
https://bencher.readthedocs.io/en/latest/
MIT License
2 stars 3 forks source link

Sweep: factor out the hash_persistent method from the sweep classes and add it to the SweepBase class #96

Closed blooop closed 1 year ago

blooop commented 1 year ago

In the file bench_vars, there are multiple classes IntSweep,FloatSweep etc, that have a hash_persistent message. Please remove all instances of that method out of those classes and add it to the SweepBase class.

sweep-ai[bot] commented 1 year ago

Here's the PR! https://github.com/dyson-ai/bencher/pull/98.

⚡ Sweep Free Trial: I used GPT-4 to create this ticket. You have 5 GPT-4 tickets left. For more GPT-4 tickets, visit our payment portal.To get Sweep to recreate this ticket, leave a comment prefixed with "sweep:" or edit the issue.

Step 1: 🔍 Code Search

I found the following snippets in your repository. I will now analyze these snippets and come up with a plan.

Some code snippets I looked at (click to expand). If some file is missing from here, you can mention the path in the ticket description. https://github.com/dyson-ai/bencher/blob/86d06e0473f6abebc567228ee6186323715626ac/bencher/bench_vars.py#L150-L344 https://github.com/dyson-ai/bencher/blob/86d06e0473f6abebc567228ee6186323715626ac/bencher/bench_vars.py#L40-L149 https://github.com/dyson-ai/bencher/blob/86d06e0473f6abebc567228ee6186323715626ac/bencher/bench_vars.py#L354-L464 https://github.com/dyson-ai/bencher/blob/86d06e0473f6abebc567228ee6186323715626ac/test/test_vars.py#L1-L69 https://github.com/dyson-ai/bencher/blob/86d06e0473f6abebc567228ee6186323715626ac/test/test_bencher.py#L313-L388

I also found the following external resources that might be helpful:

Summaries of links found in the content:

Step 2: 🧐 Snippet Analysis

From looking through the relevant snippets, I decided to make the following modifications:

File Path Proposed Changes
bencher/bench_vars.py Move the hash_persistent method to the SweepBase class. Then, remove the hash_persistent method from all the child classes (IntSweep, FloatSweep, etc.).

Step 3: 📝 Planning

I have created a plan for writing the pull request. I am now working my plan and coding the required changes to address this issue. Here is the planned pull request:

Refactor hash_persistent method and move it to SweepBase class sweep/refactor-hash-persistent

Description

This PR refactors the hash_persistent method in the bencher repository. The hash_persistent method is currently duplicated in multiple classes (IntSweep, FloatSweep, etc.), which violates the DRY (Don't Repeat Yourself) principle. To address this, the method has been moved to the SweepBase class, which is the parent class for all these classes. This ensures that all child classes inherit the hash_persistent method, eliminating code duplication.

Summary of Changes

  • Moved the hash_persistent method from the child classes (IntSweep, FloatSweep, etc.) to the SweepBase class.
  • Removed the hash_persistent method from all the child classes to avoid redundancy.
  • Ensured that all child classes inherit the hash_persistent method from the SweepBase class.

This refactoring improves code maintainability and adheres to best practices by eliminating code duplication and promoting encapsulation and abstraction.

Step 4: ⌨️ Coding

I have finished coding the issue. I am now reviewing it for completeness.

Step 5: 🔁 Code Review

Success! 🚀

To recreate the pull request, leave a comment prefixed with "sweep:" or edit the issue. Join Our Discord