CLN: Add teardowns for some benchmarks (#17616) #18388
Conversation
asv_bench/benchmarks/io_bench.py
Outdated
| self.df.to_csv(self.fname) | ||
|
|
||
| def teardown(self): | ||
| os.remove(self.fname) |
There was a problem hiding this comment.
This can raise an error if os.remove function fails. I would suggest that you try to catch any errors here in a try-except (some of the other files that you modified do this).
There was a problem hiding this comment.
I would try except all of these teardowns, maybe create a generic function at the top of this file to do so.
There was a problem hiding this comment.
yep I like that better actually. I think all of the classes that need it are in this module anyhow.
doc/source/whatsnew/v0.22.0.txt
Outdated
| @@ -167,5 +167,5 @@ Other | |||
| ^^^^^ | |||
|
|
|||
| - Improved error message when attempting to use a Python keyword as an identifier in a numexpr query (:issue:`18221`) | |||
| - | |||
| - Added teardown functions in asv benchmarks for hdfstore, io and packers benches. | |||
There was a problem hiding this comment.
@jreback @jorisvandenbossche : Do we do whatsnew for these types of changes?
There was a problem hiding this comment.
Typically not. There are a bit two conflicting aspects of the whatsnew file: informing users about changes/fixes (and then this is not needed in there as no user impact) and valuing contributors (by having their PR listed in the whathsnew note).
There was a problem hiding this comment.
Okay, just wanted to double-check. Given that is not user-facing, @dmanikowski-reef , could you revert your addition to the whatsnew ?
@jorisvandenbossche : Not sure if they have to be conflicting (e.g. could we list the PR's accepted between releases at the end of the whatsnew?) - however, can always continue this conversation outside of this PR if need be.
|
Apart from the comment of @gfyoung about the try .. except, this looks good! |
Codecov Report
@@ Coverage Diff @@
## master #18388 +/- ##
==========================================
- Coverage 91.36% 91.34% -0.02%
==========================================
Files 164 164
Lines 49718 49721 +3
==========================================
- Hits 45426 45420 -6
- Misses 4292 4301 +9
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #18388 +/- ##
==========================================
- Coverage 91.36% 91.34% -0.02%
==========================================
Files 164 164
Lines 49730 49730
==========================================
- Hits 45435 45426 -9
- Misses 4295 4304 +9
Continue to review full report at Codecov.
|
dmanikowski-reef
force-pushed
the
asv_teardown
branch
from
2656c58 to
a9ccd04
Compare
Added teardowns for hdfstore, io and packers benchmarks.
dmanikowski-reef
force-pushed
the
asv_teardown
branch
from
a9ccd04 to
ddbd767
Compare
|
Hey guys, |
|
Thanks!
Pinging (like you did) can help, as there are many PRs and we have to check them to see if something has been update or not. Or, adding new commits to the branch (instead of amending the original commit) will also generate a notification, so that can also help (and it also helps in seeing what you updated since the last review). |
Added teardown functions for hdfstore, io and packers asv benchmarks.
git diff upstream/master -u -- "*.py" | flake8 --diff