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

Exercises 1-20 #1363

Merged
merged 26 commits into from
Nov 26, 2023
Merged

Exercises 1-20 #1363

merged 26 commits into from
Nov 26, 2023

Conversation

billylanchantin
Copy link
Contributor

@billylanchantin billylanchantin commented Nov 10, 2023

Description

As discussed in Slack, this PR adds a translation of the first 20 exercises from the Python notebook 100 Numpy Exercises:

https://www.kaggle.com/code/utsav15/100-numpy-exercises/notebook

Changes

  • Adds exercises-1-20.livemd file
  • Tweaks the doc grouping in mix.exs
    Screen Shot 2023-11-10 at 5 01 14 PM

Notes

  • Most of these first 20 translate 1-1. There were a few that didn't. I left comments (which we'll want to remove before merging).
  • Going forward, not all of the exercises are going to make sense for Nx. This is (hopefully) more a jumping off point for community-curated exercises.

Copy link
Contributor

@polvalente polvalente left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work! I left some comments, but I'm really happy about the direction this guide is taking

@polvalente
Copy link
Contributor

I accidentally approved instead of just leaving a general comment haha! This is overall great, but I think we should iterate and converge on the solutions for a bit more before merging

@billylanchantin
Copy link
Contributor Author

@polvalente Ok, first pass at edits (thanks for reviewing!). A few thoughts:

  1. 20 exercises at a time may be too much to iterate on. Perhaps the next PR can just be 10.
  2. I resolved the comments that were rote. Usually I'd leave that up to the reviewer, but there are quite a few and I wanted to simplify the next round of edits.
  3. The auto-formatter kept clobbering the numbers. Every time I'd make a change, it'd reset all the numbers to 1.. I've stopped fighting it for now. Hopefully we can think of a solution before the merge.

@billylanchantin
Copy link
Contributor Author

Hey so I apologize for the churn here. I realize it makes this PR harder to review.

After some more discussion on slack, I think I'd like to reformat so solutions are wrapped in a <details> tag. That way you're not spoiled while you read.

I have these changes locally. If there are no objections, I'd like to push them:

nx-livebook-solution

@polvalente
Copy link
Contributor

Hey so I apologize for the churn here. I realize it makes this PR harder to review.

After some more discussion on slack, I think I'd like to reformat so solutions are wrapped in a <details> tag. That way you're not spoiled while you read.

I have these changes locally. If there are no objections, I'd like to push them:

nx-livebook-solution

Go ahead! This PR isn't that large that a refactor like this would make it impossible to review again

@billylanchantin
Copy link
Contributor Author

Ok, highlights:

  • Each solution is inside a <details> tag. Now you won't get spoiled.
  • All exercise wordings use the imperative mood. It felt better than some exercises being questions and others commands.
  • I use the phrase "Example solution" for my solutions since there may be more than one. It also leaves it open for us to add "Example solution 1", "Example solution 2", etc. when there are multiple valid approaches.

billylanchantin and others added 2 commits November 14, 2023 09:01
Co-authored-by: Jonatan Kłosko <[email protected]>
Co-authored-by: Jonatan Kłosko <[email protected]>
Copy link
Collaborator

@josevalim josevalim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay, I am (still) on holidays. I have dropped just two minor nits and we can ship it!

@josevalim josevalim merged commit 3210ef4 into elixir-nx:main Nov 26, 2023
@josevalim
Copy link
Collaborator

💚 💙 💜 💛 ❤️

@billylanchantin billylanchantin deleted the exercises-1-20 branch January 28, 2024 16:02
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.

6 participants