Feature/remove plot option #355
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @blooop - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| @@ -217,7 +217,7 @@ def sweep_sequential( | |||
| group_size: int = 1, | |||
| iterations: int = 1, | |||
| relationship_cb=None, | |||
| plot=None, | |||
There was a problem hiding this comment.
suggestion (code_clarification): Consider specifying default value for plot_callbacks more explicitly.
Using None as a default for a parameter expected to be a list or boolean can be ambiguous. Consider using an empty list or a specific boolean value as a default to enhance clarity and avoid potential NoneType errors.
| plot=None, | |
| plot_callbacks: List | bool = [], |
| @@ -270,7 +269,7 @@ | |||
| you want the benchmark function to be passed the repeat number | |||
| tag (str,optional): Use tags to group different benchmarks together. | |||
| run_cfg: (BenchRunCfg, optional): A config for storing how the benchmarks and run | |||
There was a problem hiding this comment.
nitpick (typo): Correct the typo and clarify the type in the docstring.
The word 'clal' should be corrected to 'call'. Also, the type hint in the docstring should match the actual parameter type (List | bool) instead of (List | False).
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #355 +/- ##
==========================================
- Coverage 88.98% 88.28% -0.71%
==========================================
Files 54 54
Lines 3140 3140
==========================================
- Hits 2794 2772 -22
- Misses 346 368 +22 ☔ View full report in Codecov by Sentry. |
No description provided.