Various testing speedups and tweaks#14072
Conversation
drammock
left a comment
drammock
left a comment
There was a problem hiding this comment.
Couple comments, I only had time for a quick skim. This might be a good one for @nordme , @CarinaFo , @scott-huberty , and @tsbinns to look at, since it deals with a topic of interest (speeding up tests)
| onset += raw.first_time | ||
| assert_array_equal(annot_nan.onset, onset) | ||
| assert_array_equal(annot_nan.duration, np.array([2])) | ||
| assert_allclose(annot_nan.onset, onset, rtol=1e-3) |
There was a problem hiding this comment.
Why the change from equal to all close here?
There was a problem hiding this comment.
Sample dataset uses non integer sampling rate. Original test resampled to 100 Hz but that makes it take much longer, and the test shows the same thing conceptually without it
| if c.nnz > 0: | ||
| out += pre + (f" sparse matrix a and b differ on {c.nnz} elements") | ||
| elif pd and isinstance(a, pd.DataFrame): | ||
| elif (pd := sys.modules.get("pandas")) is not None and isinstance(a, pd.DataFrame): |
There was a problem hiding this comment.
Would it make more sense to do this within check_pandad_installed and leave this line the way it was?
There was a problem hiding this comment.
The check will actually attempt an import, which is what we're trying to avoid (takes time if you haven't imported it yet). Don't need to check to see if the data are a data frame here if you couldn't possibly have one yet (i.e. if you haven't imported pandas yourself yet)
There was a problem hiding this comment.
... And I don't think a sys.modules check inside check_pandas_installed makes sense, it would need to be a new check_pandas_imported or add a new kwarg like imported_only or something to the existing function to make it kind of a misnomer
3 participants
Speed up tests:
ultraslowto speed up main test runsPart of #14063. Investigated speedups and drafted changes using Claude Fable 5 and Opus 4.8, then I revised (and understand + vouch for all changes).