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

new random concept #3556

Merged
merged 30 commits into from
Dec 7, 2023
Merged

new random concept #3556

merged 30 commits into from
Dec 7, 2023

Conversation

colinleach
Copy link
Contributor

As discussed, to be reviewed at your leisure.

Don't worry, they are unlikely to keep arriving at this tempo.

@BethanyG
Copy link
Member

Don't worry, they are unlikely to keep arriving at this tempo.

That's a bit of a relief, given I am 10x slower than you are! I LOVE it tho. Thank you so much for your hard work on these. 🎉 I am running behind today, but will make sure to give this a go-over, as well as taking care of the intros for Fractions and Complex Numbers. So at least you'll have something to wake up to. 😉

@colinleach
Copy link
Contributor Author

No rush. Breathe, relax, enjoy your evening...

@colinleach
Copy link
Contributor Author

colinleach commented Nov 30, 2023

I admit that my weighing-bolts story muddles the standard deviation with the standard error of the mean (a factor of $1/\sqrt{n}$, if I remember correctly), but I suspect not many Python students want to get into that.

Copy link
Member

@BethanyG BethanyG left a comment

Choose a reason for hiding this comment

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

I think this looks pretty darn good. I want to circle back and add more links, but this largely feels finished to me. Nice work, you! 🚀 🌟

I did, of course have comments...I always do. But feel free to tell me NO, or to push back where you don't agree.

concepts/random/about.md Outdated Show resolved Hide resolved
Comment on lines 39 to 46
>>> random.randrange(500)
219
>>> [random.randrange(0, 10, 2) for _ in range(10)]
[2, 8, 4, 0, 4, 2, 6, 6, 8, 8]

>>> random.randint(1, 6) # roll a die
4
```
Copy link
Member

Choose a reason for hiding this comment

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

sigh. I want to keep this here, buuuut. If this is going to be "high" on the tree, then we can't use a list comprehension here, and instead have to do loop-append. But that being said, this will also be before loops. So maybe we keep it....I will have to think about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I admit I wondered about that after submitting it, without reaching a clear conclusion.

Another thing I worry about: these are high up but absolutely depend on import. How do we resolve that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A few minutes later, with a bit more thought: this would be a killer error in introduction.md. For about.md I think we have more leeway to forward-reference concepts the students haven't reached yet. May need an explanatory comment.

Copy link
Member

Choose a reason for hiding this comment

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

Another thing I worry about: these are high up but absolutely depend on import. How do we resolve that?

Errr. Yeah. That said, import is really easy to show by example. At least that's what I tell myself every time it comes up. But if it is also raising concerns for you, we might want to brainstorm on how we address that.

One option, of course, is to move concepts around. I like the idea of keeping math related stuff in a cluster, but I also don't want to send anyone screaming for the exit. Now, none of these are on the "critical path" of prerequisites, but a lot of folx might very well go through them because they are there.

Option two would be to briefly show & explain how you do an import, and hope that we don't get complaints about the hand-waving. I am not sure we would, since its really straightforward. Something along the lines of:

 To use the `random` module, you must first import it like so:

   ```python
   import random

  random.choice([1,2,3,4,5,6,7])
  ```

Option three would be to write up an import concept. I am allergic to this for several reasons:

  1. Doing it early means skipping the (important) details around namespacing, names, and aliases.
  2. If we leave out namespacing, names, and aliases -- what is left to include that isn't just an example?
  3. Even if we do a really good job, how much of the detail will help students, and how much will they retain?

I think I might be in favor of option two. What are your thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I might be in favor of option two

Agreed, with the caveat that we may also need from xxx import yyy. I've used that a lot in datetime, though not in the numbers cluster.

Next step: when do we discourage from xxx import *. Presumably, after namespaces.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I might say immediately. But certainly after namespaces.

concepts/random/about.md Outdated Show resolved Hide resolved
concepts/random/about.md Outdated Show resolved Hide resolved
concepts/random/about.md Outdated Show resolved Hide resolved
concepts/random/about.md Outdated Show resolved Hide resolved
concepts/random/about.md Outdated Show resolved Hide resolved
concepts/random/about.md Outdated Show resolved Hide resolved
concepts/random/about.md Outdated Show resolved Hide resolved
concepts/random/about.md Outdated Show resolved Hide resolved
@BethanyG
Copy link
Member

BethanyG commented Dec 6, 2023

Looks like this PR has merge conflicts. I hazard a guess its due to a changed track config.json. A fetch followed by a rebase should do the trick -- but I can also do that when I add links, should you not want to. 😄

colinleach and others added 23 commits December 5, 2023 19:10
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
I have been avoiding $\LaTeX$ (very reluctantly, but Jeremy and Erik insist). I guess Unicode will save us here.

Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
colinleach and others added 2 commits December 5, 2023 19:21
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
@colinleach
Copy link
Contributor Author

colinleach commented Dec 6, 2023

Merge conflict should be resolved now.

@BethanyG
Copy link
Member

BethanyG commented Dec 6, 2023

Merge conflict should be resolved now.

It is! Thank you!! 💙

@BethanyG
Copy link
Member

BethanyG commented Dec 6, 2023

Alrighty. Added an introduction.md, and a few other links. Some additional light edits.
Give it a look, and LMK if it is ok, or if adjustments are needed.

If its all good, we can merge it in today. 😄

@colinleach
Copy link
Contributor Author

The preview I looked at on GitHub seemed to have some broken links, but these may not be real.

Otherwise, it looks great.

@BethanyG BethanyG merged commit 0374d1c into exercism:main Dec 7, 2023
8 checks passed
@colinleach colinleach deleted the random branch December 7, 2023 14:59
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.

2 participants