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

Practice what you preach: Variable names #725

Open
SobhanOmranian opened this issue Oct 10, 2019 · 2 comments

Comments

@SobhanOmranian
Copy link

commented Oct 10, 2019

In the lesson Repeating Actions with Loops, we have a section named "What’s in a name?", in which it is stated that although the loop variable can be arbitrarily named, it is a good practice to give it a meaningful name in order to make its purpose more clear.

This is indeed an important, yet often missed point that can pay dividends in the long term from the readability aspect of the code. As most of the audience of this lesson are novices with fresh minds and ears, it is as important that the material consistently adheres to the principles it advocates.

However, in numerous episodes throughout the lesson, we do not see proper naming for the (loop) variable names. For example, in the episode Analyzing Data from Multiple Files, there is the following piece of code (condensed for brevity):

filenames = sorted(glob.glob('inflammation*.csv'))
filenames = filenames[0:3]
for f in filenames:
    print(f)
    data = numpy.loadtxt(fname=f, delimiter=',')

As we can see, although the variable filenames is named carefully, the loop variable has lazily been abbreviated from file or filename to f. While this might easily be okay in a more advanced course since the learners are more familiar with the concept of variables and naming conventions, it is in direct contradiction with the aforementioned good practice. A learner might wonder why such practice is not followed in the course.

In fact, I do not seem to be able to find a good reason behind abbreviating such variable names. If saving time or space is the underlying reason, then typing f instead of filename hardly saves any of that. However, having meaningful names such as filename or file does make the code more readable and is more aligned with the best practices promoted earlier. I hope the lesson maintainers could shed some light on the reasoning behind choosing such variable names.

This issue is propagated to various parts of the lesson. For instance another example would be in Creating Functions:

filenames = sorted(glob.glob('inflammation*.csv'))
for f in filenames[:3]:
    print(f)
    visualize(f)
    detect_problems(f)

I can provide a list of all the instances, if the consensus is that we are in need of more descriptive variable names in such cases.

@ldko

This comment has been minimized.

Copy link
Contributor

commented Oct 10, 2019

Thanks, @SobhanOmranian. This is a valid point you have brought up. I don't think vague loop variable names were intentionally chosen for the code samples/examples. You are right; we should be carrying forward the practices that we teach to the code we are using throughout the episodes. If you want to open PRs (perhaps one per episode) to contribute changes that fix this issue, that would be great. If you prefer to list the instances instead, as you mention, we can ask others to contribute fixes for what you highlight.

@maxim-belkin

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2019

Thanks for the report, @SobhanOmranian!

I hope the lesson maintainers could shed some light on the reasoning behind choosing such variable names.

Things like that happen mostly due to the nature of the lesson development process within The Carpentries -- it is an iterative process, driven and guided by the community members. Inconsistencies like this one are easy to miss in the early stages of lesson development when a lot of new material is added.

$ grep 'for . in ' *
02-loop.md:> > for i in range(1, 4):
02-loop.md:> > for i in range(0, 3):
04-files.md:for f in filenames:
04-files.md:> for f in filenames:
04-files.md:> > for f in filenames:
06-func.md:for f in filenames[:3]:
06-func.md:    for v in p:
06-func.md:    for v in p:
08-defensive.md:for n in numbers:
08-defensive.md:      3 for n in numbers:
10-cmdline.md:    for m in numpy.mean(data, axis=1):
10-cmdline.md:    for m in numpy.mean(data, axis=1):
10-cmdline.md:        for m in numpy.mean(data, axis=1):
10-cmdline.md:    for f in filenames:
10-cmdline.md:        for m in values:
10-cmdline.md:    for f in filenames:
10-cmdline.md:    for m in values:
10-cmdline.md:        for f in filenames:
10-cmdline.md:    for m in values:
10-cmdline.md:> >         for f in filenames:
10-cmdline.md:> >     for m in values:
10-cmdline.md:> >         for f in filenames:
10-cmdline.md:> >     for m in values:
10-cmdline.md:> >         for f in filenames:
10-cmdline.md:> >     for m in values:
10-cmdline.md:> >         for f in filenames[1:]:
10-cmdline.md:> >         for f in filenames:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.