Skip to content
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

Feature/verify flag #192

Closed
wants to merge 3 commits into from
Closed

Conversation

jspaezp
Copy link

@jspaezp jspaezp commented Feb 14, 2024

No description provided.

JasonJoosteCSIRO and others added 3 commits May 5, 2023 21:23
* Add command line argument keep-id, which maintiains randomly generated cell ids. Otherwise cell ids are assigned incrementally (after the removal of cells), which should keep them consistent across runs in version control

* Modify test_cell and test_exception in test_keep_output_tags.py to use the new strip_output signature

* Fix failed test_end_to_end_nbstripout with test_max_size by passing --keep-id for keeping the existing ids

* Add tests for notebooks with and without the --keep-id flag. A new extension expected_id was added for expected output with ordered ids

* Modify the readme to include the --include-id flag

* Add keyword arguments for None inputs in test_keep_output_tags.py

* Rename expected output files to make desired sequential ids more explicit

Co-authored-by: Florian Rathgeber <florian.rathgeber@gmail.com>
@kynan
Copy link
Owner

kynan commented Feb 17, 2024

Thanks for sending this! Unfortunately this is currently very hard to review as you mix your own change with the already merged PRs #184 and #186. Can you please rebase your change on the top of the current main branch?

@kynan kynan self-requested a review February 17, 2024 12:46
@jspaezp
Copy link
Author

jspaezp commented Feb 18, 2024

I am sincerely sorry for the lack of context here! Your comment is overly nice.
This PR was not meant to be reviewed to be merged but rather to be pointed to in the issue #153. I apologize for the misunderstanding!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants