search

swcarpentry / python-novice-gapminder

Plotting and Programming in Python
http://swcarpentry.github.io/python-novice-gapminder/
Other
163 stars 428 forks source link

Added `random.choice` as sampling option (581) #586

Closed guydav closed 2 years ago

guydav commented 2 years ago

Also removed the reference to being able to find more convoluted ways to solve the problem, as suggested in the issue discussion.

Resolves #581 .

alee commented 2 years ago

Hi @guydav - this LGTM and I'm happy to merge, but for posterity you should put these changes on a branch in your fork to avoid future issues with keeping your fork and this upstream repository synced. You could always delete and re-fork but it's a good habit to always submit pull requests to upstream from a new branch in your fork.

guydav commented 2 years ago

Hi @alee -- I don't know how I forgot to do that, my apologies. Would you prefer if resubmit the PR in a separate branch?

alee commented 2 years ago

If you have the time and would like the GitHub practice that's fine, otherwise just wanted you to be forewarned that after merging your fork's gh-pages and this upstream will be out of sync and you'll likely have to do some Git surgery to get things lined up again

guydav commented 2 years ago

I think I can handle that, that sounds reasonable enough.

On Thu, Feb 24, 2022 at 10:47 AM A Lee @.***> wrote:

If you have the time and would like the GitHub practice that's fine, otherwise just wanted you to be forewarned that after merging your fork's gh-pages and this upstream will be out of sync and you'll likely have to do some Git surgery to get things lined up again

— Reply to this email directly, view it on GitHub https://github.com/swcarpentry/python-novice-gapminder/pull/586#issuecomment-1049993327, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALFDFEDZVE4XQADWT7YX5TU4ZHHNANCNFSM5MFGXVAA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

alee commented 2 years ago

Great! You should be able to do it straight from the GitHub web interface by clicking on the branch dropdown at the top left of the Code tab:

image

As soon as the branch is created you can submit the PR from it. Don't worry about bringing the branch up to date with upstream, I don't think there are any conflicts.

After that's done you'll still need to reset your fork's gh-pages to bring it in line with upstream though..

guydav commented 2 years ago

Done. See #592