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

Perhaps we shouldn't have readings-01.py, readings-02.py etc? #63

Open
DamienIrving opened this issue Apr 18, 2015 · 20 comments
Open

Perhaps we shouldn't have readings-01.py, readings-02.py etc? #63

DamienIrving opened this issue Apr 18, 2015 · 20 comments
Assignees

Comments

@DamienIrving
Copy link
Contributor

@DamienIrving DamienIrving commented Apr 18, 2015

Every time I read the command line lesson (08-cmdline.html), the files readings-01.py, readings-02.py, etc make me very uncomfortable, because that type of file naming behavior is exactly what we are trying to avoid by teaching version control.

I think the solution to this problem comes from the very beginning of the lesson, which reads:

... save the following in a text file called sys-version.py:

import sys
print 'version is', sys.version

We could take a similar approach to the rest of the lesson and instead of saying $ cat readings-01.py we could simply save the following in a text file called readings.py:

import sys
import numpy as np

def main():
    script = sys.argv[0]
    filename = sys.argv[1]
    data = np.loadtxt(filename, delimiter=',')
    for m in data.mean(axis=1):
        print m

Later in the lesson when we get to readings-02.py, we could just say that we open our text editor and make the following changes to readings.py:

import sys
import numpy as np

def main():
    script = sys.argv[0]
    filename = sys.argv[1]
    data = np.loadtxt(filename, delimiter=',')
    for m in data.mean(axis=1):
        print m

main()

The trick would be to somehow highlight main() in bold or a different color, because that is the part of the script that has changed since last time (i.e. kind of like git diff highlighting).

If people are happy with this suggestion of altering the wording of the command line lesson to make successive edits to readings.py as opposed to going $ cat readings-01.py, $ cat readings-02.py etc then I'd be happy to submit a pull request. (I'm also open to suggestions of how to incorporate git diff style highlighting into that PR, but I'd have to be able to write it in markdown and then and have pandoc convert it appropriately - I'm not aware of any way to make that happen.)

@wking
Copy link
Member

@wking wking commented Apr 18, 2015

On Fri, Apr 17, 2015 at 08:52:20PM -0700, Damien Irving wrote:

If people are happy with this suggestion of altering the wording of
the command line lesson to make successive edits to readings.py as
opposed to going $ cat readings-01.py, $ cat readings-02.py etc
then I'd be happy to submit a pull request.

I like the idea, and if we could always count on folks learning Git
(or some other VCS) before they hit this lesson, I'd just put the
readings.py in a little repository of its own. But since some folks
teach Python before Git (or another VCS), and some don't even teach
VCSes at all (some Data Carpentry stuff?), I'm a bit concerned about
jumping in so thoroughly. In order of increasing murkiness, I think
it's:

  1. Version controled readings.py (if you've been taught version
    control)
  2. Numbered readings-XX.py like we have now
  3. “… and then edit it again …” like you're proposing
  4. Version controled readings.py (if you haven't been taught version
    control)

But (3) is such a good excuse to remind version-control-aware folks to
commit their changes that I think it's worth the slight increase in
murkiness for folks that haven't had version control yet.

@DamienIrving
Copy link
Contributor Author

@DamienIrving DamienIrving commented Apr 18, 2015

@wking Totally agree that most people (including myself) teach python before git, so I definitely don't think we should go down the route of having a version controlled copy of readings.py. I'm simply proposing your number (3) - we (and I'm happy to do this and submit a PR) simply edit the notes to say that we are making successive edits to readings.py, as opposed to having separate files for each edit.

@wking
Copy link
Member

@wking wking commented Apr 18, 2015

On Fri, Apr 17, 2015 at 09:09:19PM -0700, Damien Irving wrote:

I'm simply proposing your number (3) - we (and I'm happy to do this
and submit a PR) simply edit the notes to say that we are making
successive edits to readings.py, as opposed to having separate
files for each edit.

Right. I was just laying out the alternatives and their relative pros
and cons (as I see them). As I said in that comment, I think (3) is a
bit less clear for folks who haven't learned about version control
yet, but it's a great opportunity for folks who have learned about
version control to use their knew knowledge. So I think a PR
switching to this approach would be a net gain in lesson usefulness.
I'm not the maintainer, so it's not my call, but that's my 2¢ ;).

@kynan
Copy link
Contributor

@kynan kynan commented Apr 19, 2015

Agreed that this is a good opportunity to rehash version control if/where appropriate. And if not, a 2 line change to a file is probably straightforward enough to not warrant an extra copy even when not using a VCS.

@gvwilson
Copy link
Member

@gvwilson gvwilson commented Apr 19, 2015

@kynan
Copy link
Contributor

@kynan kynan commented Apr 19, 2015

I wouldn't mention VCS in the text. The instructors can do that orally if appropriate.

@wking
Copy link
Member

@wking wking commented Apr 20, 2015

On Sun, Apr 19, 2015 at 04:55:05AM -0700, Florian Rathgeber wrote:

I wouldn't mention VCS in the text. The instructors can do that
orally if appropriate.

+1

@abostroem
Copy link
Contributor

@abostroem abostroem commented Apr 20, 2015

I'd love some more comments from people who've taught this recently. Is the suggested code by @DamienIrving at an appropriate level for your students? Too complicated? Just right?

@DamienIrving
Copy link
Contributor Author

@DamienIrving DamienIrving commented Apr 20, 2015

@abostroem I think this conversation is getting a little confused about exactly what I was suggesting. All I want to do is remove the readings-01.py, readings-02.py, etc files from the code directory because it is suggestive of a workflow that we don't want to promote. The actual content of the lesson won't change at all. To make myself more explicit, I'm thinking I'll just submit a PR with the proposed changes once my pending PR (#62) has been dealt with.

@abostroem
Copy link
Contributor

@abostroem abostroem commented Apr 20, 2015

@DamienIrving Your suggestion is to modify the reading-01.py file in place (and maybe indicate this in the instructions somewhere) rather than create a new file with poor naming convention?

@DamienIrving
Copy link
Contributor Author

@DamienIrving DamienIrving commented Apr 20, 2015

@abostroem Exactly (and yes to indicating in the instructions). I have no intention of mentioning version control in this lesson - I just want to avoid exposure to the poor naming convention because that is inconsistent with what we teach in the version control lesson.

@DamienIrving
Copy link
Contributor Author

@DamienIrving DamienIrving commented Apr 20, 2015

A nice little addition would be to highlight the changes from one in place edit to the next (e.g. new text in bold), however I have no idea how to make text bold within a block of markdown code.

@abostroem
Copy link
Contributor

@abostroem abostroem commented Apr 20, 2015

Sounds good to me. Go ahead and make the changes to the file structure and instructions. My only concern is that it is clear where and what we are changing with each iteration.

@DamienIrving
Copy link
Contributor Author

@DamienIrving DamienIrving commented Apr 20, 2015

Yeah, that's my concern too. readings.py is fairly short, so hopefully with some edits to the supporting text the changes will be obvious. If not, you can always just reject my PR :-)

@gvwilson
Copy link
Member

@gvwilson gvwilson commented Jul 31, 2016

Happy to rename if someone would like to PR a name change.

@maxim-belkin
Copy link
Contributor

@maxim-belkin maxim-belkin commented Jan 17, 2020

This issue is the oldest open issue in our lesson, where consensus has been reached.

I think it makes sense for an instructor to say "and now let's modify our script to do ..." during the lesson. But, perhaps, it also makes sense to have these scripts somewhere in the repo (e.g., in a new "solutions" subdirectory).

Anne (@annefou), Lauren (@ldko), what do you think?

@ldko
Copy link
Contributor

@ldko ldko commented Jan 17, 2020

I agree with the instructor having learners start the code and save it in a file called readings.py and indicate to modify this same code as the episode progresses. I like the suggestion of showing the script throughout the lesson with the changing lines highlighted. I am not a fan of the idea of saving all the readings_*.py files in the repo. Similar to how the code subdirectory has a copy of the sys_version.py script that learners create in the Command-Line Programs episode, I think it would make sense to have a finished copy of the readings.py file there as well.

@annefou
Copy link
Contributor

@annefou annefou commented Jan 17, 2020

Yes. For me it makes sense to keep readings.py and modify the same code step by step. The progression (from the "initial" version to the "final" version) is helpful for learners but the numbering readings-01.py, readings-02.py is artificial and indeed goes against what we are trying to teach in the Carpentries.

And I also agree that we can have the final version of readings.py in the code subdirectory.

@maxim-belkin
Copy link
Contributor

@maxim-belkin maxim-belkin commented Jan 17, 2020

Thank you, Lauren and Anne.

Damien (@DamienIrving), are you still interested in working on this issue or would you prefer to delegate it to us / someone else?

@DamienIrving
Copy link
Contributor Author

@DamienIrving DamienIrving commented Jan 20, 2020

@maxim-belkin Sorry to be that person, but if I could delegate that would be great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
9 participants
You can’t perform that action at this time.