-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added skip feature #142
Added skip feature #142
Conversation
@SagiPolaczek Thanks for raising this PR! I apologize for not having reviewed this yet, I will jump on this ASAP. |
I wonder why CI hasn't been triggered on this PR. Strange. |
@SagiPolaczek can you please try pushing an empty commit using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments.
Also, please add some unit tests to testbook/tests/test_client.py
for when skip
is: int
, str
, list[int]
, [list[str]
.
Thanks again!
if isinstance(skip, int): | ||
skip = [skip] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would also want to accept cell tag(s) as input for the skip
argument for users who wouldn't want to concern themselves with knowing which index each cell is at.
if isinstance(skip, int): | |
skip = [skip] | |
if isinstance(skip, int) or isinstance(skip, str): | |
skip = [skip] | |
if all(isinstance(x, str) for x in skip): | |
skip = [self._cell_index(tag) for tag in skip] | |
""" | ||
Executes all cells | ||
Executes all cells besides the ones who were asked to be skipped |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Executes all cells besides the ones who were asked to be skipped | |
Executes all cells. | |
If skip is passed, we will remove those cell(s) from the list of cells to be executed. |
@rohitsanj I kinda want this feature so if it's cool with you can I take this across the finish line? |
Sure, @rprater! Thanks. |
Sorry, I totally missed it and moved on! Thanks @rohitsanj for taking it and thanks @rohitsanj for maintaining the repo. |
I'm closing all of my inactive PRs. Thanks! |
#141
This is the implementation I came up with. See details of the feature in the attached issue