Closed ghost closed 1 year ago
Should all be addressed right now. The merge in the middle is a little messy but I'm not sure its worth the wrangling. The zeppelin strip is in its own function so wasn't affected anyway.
Thanks for your contribution!
My pleasure 🙂
Jason Jooste
Machine Learning Operations Engineer
Data61 | CSIRO
@.**@.>
From: Florian Rathgeber @.> Sent: 06 May 2023 05:24 To: kynan/nbstripout @.> Cc: Jooste, Jason (Data61, Pullenvale) @.>; Author @.> Subject: Re: [kynan/nbstripout] Ordered cell ids (PR #184)
Thanks for your contribution!
— Reply to this email directly, view it on GitHubhttps://github.com/kynan/nbstripout/pull/184#issuecomment-1536678324, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A4YFHRNXUNTY54JAGKFV2ZDXEVHVFANCNFSM6AAAAAAXMDVUMU. You are receiving this because you authored the thread.Message ID: @.***>
This PR fixes a problem that I've been having with cell IDs not being consistent, even if the cell contents remain the same. It seems to have something to do with the new nbformat. Simply removing cell.id gives a warning which in future versions will be a hard exception.
The solution that I have implemented that I think is most appropriate is to just add incremental cell id's (which is consistent with the intended use of the ids. Since IDs are usually randomly generated and most users aren't exposed to them I think this is a good solution.
I've added a new command line flag --keep-id to turn off this behaviour. My guess being on by default would match the expectations of most people.
I've also added a couple of tests for this new format.
Questions:
This is my first time contributing to an open source project like this and I'm not entirely sure about the etiquette in these situations. Let me know if I could do anything more!
Cheers, Jason