swcarpentry / python-novice-inflammation

Programming with Python
http://swcarpentry.github.io/python-novice-inflammation/
Other
303 stars 781 forks source link

05-suggesting a more Pythonic approach to copying #834

Open ddefranza opened 4 years ago

ddefranza commented 4 years ago

In lesson Programming with Python, episode 5, "Storing Multiple Values in Lists" there is a very important discussion of making a copy of a list. This is demonstrated by literally creating a new list from the variable assigned to the old list, e.g.:

my_salsa = list(salsa)

This is fine, as is using slicing to do the same. However, the more Pythonic way to accomplish this is by using the copy method, e.g.:

my_salsa = salsa.copy()

Thanks!

ldko commented 4 years ago

Hi @ddefranza , Having come up in Python 2 programming, where the copy method was not available, I pretty much never think to use it :grin: , but it makes sense to me to teach copying the list with copy rather than the list function because I think it's name would make more sense and be more memorable to new learners.

There is also a PR #715 from a while back proposing to use copy.deepcopy() instead of list() which would also cover copying a list containing mutable objects, however there was some leaning to this introducing too much cognitive load.

Let's see if anyone else from the community wants to offer an opinion about copy method versus list function. If there is agreement toward copy perhaps you could offer a PR to change the couple of instances of list() used for copying in the episode. Thanks for making this suggestion!

ddefranza commented 4 years ago

Sounds good. Introducing the copy method here, in an early discussion of lists, might lay the groundwork for introducing copy.deepcopy() later on. Thanks!

ldko commented 4 years ago

No one else has chimed in @ddefranza , so if you are still available to make the salsa.copy() change, please open a pull request at your convenience. Thank you!

maxim-belkin commented 4 years ago

No one else has chimed in @ddefranza

I apologize for missing this issue -- please feel free to @-mention me directly or with @swcarpentry/python-novice-inflammation-maintainers.

I agree with laying the groundwork for introducing the copy.deepcopy() method but shall we use copy.copy() as an intermediate step rather than salsa.copy()? This way we would keep using the modulename.method() scheme rather than object.method().

ldko commented 4 years ago

@maxim-belkin we use other list methods in this episode (append, pop, reverse), so it seemed to me that object.method() would be ok to use here. If we want to use modulename.function() then I would prefer skipping copy.copy() altogether and only teach copy.deepcopy(), since it comes from the same module and will give you the same result as copy.copy() in this case but also handle a list containing mutable objects (this would save on time by avoiding teaching multiple ways of doing the same thing and perhaps help prevent overload for learners). If you think we should avoid object.method() and don't want to teach copy.deepcopy() on its own, I would rather stick with list() for copying over copy.copy(), since it doesn't require an import. Of course, that is my opinion, and I will go with the consensus of the community.

maxim-belkin commented 4 years ago

I'm not opposed to adding object.copy() -- I actually tend to agree that we need it -- but we need to be careful and consider all pros and cons beyond our personal preferences and justifications based on whether something is "Pythonic" or not. After all, "Pythonic" refers to a way of carrying out programming procedures in Python (which may be optimal and easy to understand) but not to the "only right way" of doing stuff. We need to be considerate of our lesson needs, think how to blend it (new material) into the main story line, how it is going to change the load on the learners, etc. From my experience teaching this part, I always felt that it doesn't provide sufficient motivation -- new knowledge is declared rather than discovered.

So, I think the right thing to do here is to implement the change that David suggests -- again, I agree that .copy() is a useful bit of information. At the same time, I suggest to begin the discussion of how to improve / modify this part of the lesson so that it flows better and doesn't give an impression of a bag of information.