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

Save cleared notebooks as target version #5

Merged
merged 2 commits into from
Aug 16, 2017
Merged

Save cleared notebooks as target version #5

merged 2 commits into from
Aug 16, 2017

Conversation

tbekolay
Copy link
Member

I did this as requested in nengo/nengo#1129 to save all the notebooks as version 3 ... however, the result is not great because whether you set format 3 or 4, all notebooks in the Nengo repo will be changed. I think there's some disparity between what nbconvert and nbformat are doing and what the actual notebook are doing... but yeah, I don't use this script so I don't think it's worth tracking down the disparity. Leaving this here in any case if anyone finds it useful.

Currently the target is version 3.
@jgosmann
Copy link
Contributor

I would assume it changes all the notebooks only once (i.e. this is idempotent, it doesn't change the notebooks more than once with repeated application)?

@tbekolay
Copy link
Member Author

I believe so. I don't think it's worth doing this for all notebooks at the moment, though perhaps when we upgrade them all to v4.

@Seanny123
Copy link

LGTM.

@Seanny123
Copy link

Any concerns that need to be addressed before this is merged?

@jgosmann
Copy link
Contributor

Added a commit to add a command line argument for the target version. I think this should be merged once that commit gets a LGTM.

@Seanny123
Copy link

LGTM.

@jgosmann jgosmann merged commit fad219c into master Aug 16, 2017
@tbekolay tbekolay deleted the changeversion branch August 16, 2017 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants