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

Small fixes #9

Merged
merged 11 commits into from
Jun 28, 2021
Merged

Small fixes #9

merged 11 commits into from
Jun 28, 2021

Conversation

lukestein
Copy link
Contributor

I'm enjoying this great resource so much; thank you @aeturrell! I've been fixing a few small things as I spot them (some aesthetic, but some substantive), but realized that VS Code on my end seems to be reordering the metadata and outputs on every cell, which makes the diffs look terrible.

I therefore wanted to submit this PR quickly before you made any upstream commits. Feel free of course to deny this PR if you'd prefer, and I can try to just make a list of substantive things as I spot them rather than update my fork.

@lukestein
Copy link
Contributor Author

Ok seems like https://nbdime.readthedocs.io/en/latest/ may be the trick for us

@lukestein
Copy link
Contributor Author

And… we're in business! It seems like VSCode and nbstripout save the JSON metadata fields in a different order. Once I started running your pre-commit hooks---I'm going to learn a bit more git come hell or high water, and had never run pre-commit before---the diffs look clean.

@aeturrell
Copy link
Owner

@lukestein thank you so much for making the first PR for "coding for economists" 🎉 (Also reminds me that I need to write a contributors section!)

This PR looks good now apart from one thing: in code-basics.ipynb, the ids have all changed, which has made the diff very large compared to the number of substantive changes in either code or text cells. Looking around the internet, this might have been an issue with JupyterLab (if you used that) or another tool (because ids are quite new). Do you think you'd be able to make a commit using the original code-basics.ipynb ids?

The simplest way to do this is probably to download the original .ipynb file yourself, open it in VS Code or an updated JupyterLab (I believe the issue has now been fixed), copy and paste your changes in, and then commit the revised version.

BTW this is not a serious problem by any means (no-one sees the ids), it just creates very large diffs, which it's best to avoid if possible. Let me know if you hit a block and either I can edit or we'll just merge it in anyway 👍

@lukestein
Copy link
Contributor Author

@aeturrell , not sure why that seems to be the one file with that cell-id issue, and the diff on that file gets bad if I run it through the pre-commit hooks. But I just made the changes manually and didn't let the nbstripout run, so now the diff should be clean.

Btw my only change there was that it was unclear why you ran lower() before title(), but if I misunderstood the code/description, sorry!

@aeturrell aeturrell merged commit 2f14e0a into aeturrell:main Jun 28, 2021
@aeturrell
Copy link
Owner

🎉

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

2 participants