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

Update 12-cmdline.md: clarifying goals and some text #828

Open
wants to merge 1 commit into
base: gh-pages
Choose a base branch
from

Conversation

jennynakai
Copy link

@jennynakai jennynakai commented May 28, 2020

I added text at L24-28 clarifying the goals in the lesson, L85-88 adding detail on the sys library, and L127-128 adding detail on argv. I also suggest explaining the use of 'cat' at L165 to view files vs. editing them. These suggestions are based on points of confusion when I was going through the lesson for the live coding demo.

This is for the SWC checkout process. I added text at L24-28 clarifying the goals in the lesson, L85-88 adding detail on the sys library, and L127-128 adding detail on argv. I also suggest explaining the use of 'cat' at L165 to view files vs. editing them. These suggestions are based on points of confusion when I was going through the lesson for the live coding demo.
Copy link
Contributor

@ldko ldko left a comment

Hi @jennynakai, congratulations on your first pull request! :) I have commented with my opinion on your suggested changes. Please let me know what you think. Thank you for submitting this contribution!

and prints the average inflammation per patient. One of our goals is to produce a program which prints
the average inflammation per patient in a given file using command line arguments and flags.
The second goal is to use shell commands and flags to look at a minimum of the first four lines.
A third goal is to find the maximum inflammation in different inflammation files.
Copy link
Contributor

@ldko ldko Jun 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tend to think of these programs/commands listed in the text you provide here as examples that lead us to our goals (rather than the goals themselves) which I consider to be the Objectives listed in the episode Overview. Between what is given in the Objectives section and the text given before each of the examples (i.e. "prints the average inflammation per patient for a given file", "might also want to look at the minimum of the first four lines", "or the maximum inflammations in several files one after another"), how do you see this change expanding on what was there previously?

Copy link
Author

@jennynakai jennynakai Jun 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think my initial confusion came from jumping right into using the command line with flags when the lesson hadn't started yet. It was abrupt and didn't flow, there was no introduction. There may be a better way to address this without restating the goals, but I can't tell you what that is.

Copy link
Contributor

@ldko ldko Jun 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you mean. When taught in a workshop, this lesson usually comes after the Unix Shell lesson where learners see commands with arguments and flags, which maybe makes this less jarring for some? Perhaps to help ease into the appearance of arguments and flags in the episode, we could modify the first paragraph to include something about them by adding to this sentence:

In order to do that, we need to make our programs work like other Unix command-line tools, allowing us to use command-line arguments and flags to supply values used in the program execution.

or maybe for another possibility, add the second sentence here:

In order to do that, we need to make our programs work like other Unix command-line tools. Let's look at how we can use command-line arguments and flags to supply values to our programs.

Do you think this would help clarify things somewhat?

write a simple script that will tell us what version of
python we are using. The sys library contains variables and functions
associated with the interpreter.
Save the following in a text file called `sys_version.py`:
Copy link
Contributor

@ldko ldko Jun 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense to give a quick explanation of what the sys_version.py script will do as you have added here. I suggest a few edits: removing the word "simple" in case it is demotivating for anyone who does not consider this is simple, capitalize Python, move the description of sys library to where it is talked about a little later and tweak it slightly. What do you think about something like:

Using the text editor of your choice, let's write a
short script that tells us what version of Python we are using.
Save the following in a text file called `sys_version.py`:

Then after the code:

The first line imports a library called `sys`, which is short for “system”.
`sys` provides access to variables and functions associated with the interpreter.
It defines values...

Copy link
Author

@jennynakai jennynakai Jun 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree.

@@ -118,6 +124,8 @@ Whenever Python runs a program,
it takes all of the values given on the command line
and puts them in the list `sys.argv`
so that the program can determine what they were.
Our script name is the first argument after `python` on the
command line, therefore it is assigned to the variable argv[0].
Copy link
Contributor

@ldko ldko Jun 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if this adds to the sys.argv[0] explanation below. It may be confusing that here it is referring to the script name as an argument, but sys.argv is explained as the arguments passed to the script (implying that though the path or name is in sys.argv[0] it is not considered an argument in the context of arguments we are passing to the script we are calling). Does that make sense?

Copy link
Author

@jennynakai jennynakai Jun 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You know, I don't really know, but thanks for addressing the comments!

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

Successfully merging this pull request may close these issues.

None yet

2 participants