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

docs(guides): update ibis-for-dplyr-users.ipynb with latest #6129

Merged
merged 1 commit into from
May 3, 2023

Conversation

amoeba
Copy link
Contributor

@amoeba amoeba commented May 2, 2023

Fixes #6125

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@amoeba
Copy link
Contributor Author

amoeba commented May 2, 2023

Hey @cpcloud or @gforsyth can you take a look here? As I mentioned in #6125, I totally messed up my squash and didn't catch my mistake until #6050 was merged. I've successfully pulled the correct copy out of the reflog. While reviewing locally, I found a few issues which are also included here:

  • The TOC was actually broken and I hadn't noticed so not all headers were showing. I found the cause and fixed it so all headers show in the TOC
  • The breaking change introduced in 3caf3a1 that changes how suffixes work in joins affected this notebook and I've updated the text and code to match the new behavior
  • I updated two headers to match the same style as the previous ones so the headers are all consistent now
  • One cell had slightly quirky formatting which I've fixed

I don't think any of these changes will be controversial. I've checked that the output is right locally and on ReviewNB. Sorry for the mistake in the first place.

@cpcloud cpcloud added this to the 6.0 milestone May 2, 2023
@cpcloud cpcloud added the docs Documentation related issues or PRs label May 2, 2023
@cpcloud
Copy link
Member

cpcloud commented May 2, 2023

@amoeba This looks really great! Thanks for following up.

Couple of micronits, but nothing blocking.

@amoeba
Copy link
Contributor Author

amoeba commented May 3, 2023

Thanks @cpcloud, left you one followup question.

@cpcloud cpcloud merged commit 1aa172e into ibis-project:master May 3, 2023
@cpcloud
Copy link
Member

cpcloud commented May 3, 2023

Thanks @amoeba !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation related issues or PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docs: Fix my bad rebase of Ibis for dplyr users doc
2 participants