feat: Add support for -x/--exclude-lines.#58
Open
DanCardin wants to merge 1 commit intoplasma-umass:mainfrom
Open
feat: Add support for -x/--exclude-lines.#58DanCardin wants to merge 1 commit intoplasma-umass:mainfrom
DanCardin wants to merge 1 commit intoplasma-umass:mainfrom
Conversation
DanCardin
commented
Oct 19, 2024
| f.__code__ = self.replace_map[f.__code__] | ||
|
|
||
| frame = frame.f_back # type: ignore[assignment] | ||
| @functools.lru_cache(None) |
Author
There was a problem hiding this comment.
without this cache, I saw a ~2s slowdown relative to the current impl.
5.9s without exclusions
6.1s with exclusions and cache
8.8s with exclusions no cache
10.5s with coverage
jaltmayerpizzorno
requested changes
Oct 23, 2024
Collaborator
jaltmayerpizzorno
left a comment
There was a problem hiding this comment.
Hi, thank you for your pull request! Please check out my questions and/or make the changes requested.
a4e4379 to
a447abe
Compare
Author
|
Sorry, I think my original commit must have had some editor auto-formatting or something going on. I definitely was not consciously going through and removing annotations and altering those version-constrained blocks! Updated to be (hopefully now) only the actual changes related to the enhancement. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Attempts to fix #26
I integrated the regex matching into the line-inclusion loop (Thus it works by excluding excluded lines from the total set), but it occurred to me afterwards, that it could perhaps instead be integrated similarly to
_add_unseen_source_files. There, you're already scanning a bunch of files, compiling them, and adding their lines. It could work in reverse, where it scans all the files, compiles them, and adds matching lines to the "seen" set.I'm not familiar enough with the code to know if one would make more or less sense than the other, but I think the way I have it implemented now is at least intelligible and seems to be performant.
I tried to defer imports to expensive modules (like re/inspect) up to the point of the
exclude_linesfeature being utilized. I dont see any slowdown when not making use of the feature, but a ~0.2s slowdown when making use of the feature on a ~6s test run (~10k LOC, since I figure it scales worse with numbers of code objects scanned).I wasn't able to get all the tests running locally, it seems unrelated to my changes, and that it might be some environment specific thing that I'm not sure what I need to do; but I can't say for certain.
Also, I dont love this feature being purely given by command line options. if you're into this feature, i would ideally like to follow it up by (even optional) reading from
pyproject.tomlfor atool.slipcoversetting (for this at a minimum)