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

03-lists.md: update explanation of list references #637

Open
wants to merge 2 commits into
base: gh-pages
from

Conversation

Projects
None yet
4 participants
@davidbenncsiro
Copy link
Contributor

commented Apr 10, 2019

The suggested code change for odds and primes would still serve the intended purpose of mutating a referenced vs copied list (replacing a member of the list rather than appending), while being consistent with the generally accepted definition of primality in which 1 is normally not considered a prime. In the case of reference we'll see:

primes: [2, 3, 5, 7]
odds: [2, 3, 5, 7]

and in the case of a copied list we'll see:

primes: [2, 3, 5, 7]
odds: [1, 3, 5, 7]

Please delete this line and the text below before submitting your contribution.


Thanks for contributing! If this contribution is for instructor training, please send an email to checkout@carpentries.org with a link to this contribution so we can record your progress. You’ve completed your contribution step for instructor checkout just by submitting this contribution.

Please keep in mind that lesson maintainers are volunteers and it may be some time before they can respond to your contribution. Although not all contributions can be incorporated into the lesson materials, we appreciate your time and effort to improve the curriculum. If you have any questions about the lesson maintenance process or would like to volunteer your time as a contribution reviewer, please contact Kate Hertweck (k8hertweck@gmail.com).


Update 03-lists.md
The suggested code change for odds and primes would still serve the intended purpose of mutating a referenced vs copied list (replacing a member of the list rather than appending), while being consistent with the generally accepted of primality in which 1 is normally not considered a prime. In the case of reference we'll see:

primes: [2, 3, 5, 7]
odds: [2, 3, 5, 7]

and in the case of a copied list we'll see:

primes: [2, 3, 5, 7]
odds: [1, 3, 5, 7]
@maxim-belkin

This comment has been minimized.

Copy link
Collaborator

commented Apr 18, 2019

Thanks, @davidbenncsiro! I like it but I'd like @annefou to weigh in as well!

@maxim-belkin maxim-belkin changed the title Update 03-lists.md 03-lists.md: update explanation of list references Apr 18, 2019

@annefou

This comment has been minimized.

Copy link
Collaborator

commented Apr 18, 2019

Sorry but shouldn't we change the output too? I still see the previous outputs (when appending).

primes: [1, 3, 5, 7, 2]
odds: [1, 3, 5, 7, 2]

@maxim-belkin

This comment has been minimized.

Copy link
Collaborator

commented Apr 18, 2019

You're right, @annefou, output cells below the changed lines have to be corrected.

@davidbenncsiro

This comment has been minimized.

Copy link
Contributor Author

commented Apr 19, 2019

Indeed. I only showed the new expected outputs in my pull request comment, and failed to include them in the commit. Agreed, the outputs need to be changed as well.

Thanks @maxim-belkin & @annefou

@maxim-belkin

This comment has been minimized.

Copy link
Collaborator

commented May 1, 2019

ping, @davidbenncsiro. Did you want to update the PR yourself or did you want us to do that?

@davidbenncsiro

This comment has been minimized.

Copy link
Contributor Author

commented May 3, 2019

@maxim-belkin I'll update it; thanks for the ping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.