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

dry run seems to always print that a file would be changed #152

Open
alonme opened this issue Jun 8, 2021 · 11 comments
Open

dry run seems to always print that a file would be changed #152

alonme opened this issue Jun 8, 2021 · 11 comments

Comments

@alonme
Copy link

alonme commented Jun 8, 2021

I noticed that when using dry_run, all files are always printed as if they would be changed.

Looking at the code briefly, it does seem like this is what happens.

@kynan
Copy link
Owner

kynan commented Jun 8, 2021

all files are always printed as if they would be changed

Can you clarify what you mean? What are "all files" here?

@alonme
Copy link
Author

alonme commented Jun 9, 2021

Yes, sorry for being unclear.

every file that is passed to nbstripout will print.
Dry run: would have stripped {file}

this happens even if a strip a file and then right after that run a dry run.

my expectation is that only files that would have been changed if a "wet run" would have been used - would be printed

@kynan
Copy link
Owner

kynan commented Jun 9, 2021

OK, I see what you mean. It is a fair point, but not trivial to support unfortunately: it would mean checking if applying nbstripout would have created a diff, which we don't currently have an easy way to do.

I'll add it as a feature request to the backlog.

@alonme
Copy link
Author

alonme commented Jun 9, 2021

Thanks.

Just so i understand - what is the current value of the dry-run option?

@kynan
Copy link
Owner

kynan commented Jun 12, 2021

It will tell you which files are in scope / would be touched. I agree that your definition is more useful.

@joaonc
Copy link

joaonc commented Nov 1, 2021

Could this be related to this other issue? #160

@kynan
Copy link
Owner

kynan commented Jan 2, 2022

@joaonc only tangentially. This request is about turning --dry-run into an "actual" dry run and only report if any changes would have been made.

@kynan
Copy link
Owner

kynan commented Sep 24, 2022

@alonme Would you be interested in implementing this?

@swateek
Copy link

swateek commented Oct 6, 2022

I am facing trouble here, I'll want to give this a try at implementing this @kynan

@kynan
Copy link
Owner

kynan commented Oct 8, 2022

@swateek sure! You'd need to change this line to not overwrite the notebook and then in the following if args.dry_run: block you'd need to check if any changes were applied and then only output if there is a diff. You should also do this for the Zeppelin notebook type above. Is this enough to get you started?

@swateek
Copy link

swateek commented Oct 10, 2022

let me try this out!

swateek added a commit to swateek/nbstripout that referenced this issue Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants