Skip to content

Conversation

@nsoranzo
Copy link
Contributor

No description provided.

@abostroem
Copy link
Contributor

@nsoranzo Thanks for your contribution. The use of sorted overlaps with #227. Can you collaborate with to combine the use of the sorted function with your modifications to the text? Commenting inline on the text changes you made.

The `glob` library contains a single function, also called `glob`,
that finds files whose names match a pattern.
The `glob` library contains a function, also called `glob`,
that finds files and directories whose names match a pattern.
Copy link
Contributor

Choose a reason for hiding this comment

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

To me this makes it sound like it will return directories in the list as well (like ls) rather than files and their paths. Can you come up with a clearer way to say this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

glob.glob() does return both files and directories, not sure what you are asking me to clarify here.

@tbekolay
Copy link
Contributor

This change was also proposed in #153; from that thread:

@gvwilson:

Please modify to show glob.glob first (with items not sorted), then introduce sorted in a callout box with a note saying "you should only bother to sort if you really care, and you usually only really care as the last step of processing".

@tbekolay
Copy link
Contributor

Could you make the comment as suggested by @gvwilson above, in a callout box?

@nsoranzo
Copy link
Contributor Author

nsoranzo commented Jun 14, 2016

@tbekolay I like it as it is now, I see no need for a callout box.

@tbekolay
Copy link
Contributor

Hm, reading it over again it does look like a pretty minimal set of changes to introduce both glob and sorted, so I'm 👍 on this. Does anyone object to merging this? I'll give the rest of the day for people to respond and merge tomorrow morning if there are no objections.

@tbekolay tbekolay merged commit ca94cca into swcarpentry:gh-pages Jun 15, 2016
@tbekolay
Copy link
Contributor

Thanks for your contribution @nsoranzo!

@nsoranzo nsoranzo deleted the patch-2 branch May 2, 2017 19:21
rgaiacs pushed a commit to rgaiacs/swc-python-novice-inflammation that referenced this pull request May 6, 2017
rgaiacs pushed a commit to rgaiacs/swc-python-novice-inflammation that referenced this pull request May 6, 2017
…-readings

Changing preliminary readings
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.

3 participants