Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upCopyediting #22
Conversation
ruebot
added some commits
Mar 5, 2019
ruebot
reviewed
Mar 5, 2019
| " :param by: \"all\", \"domain\" or \"year\" the output to return\n", | ||
| " :param minline: the minimum size of a line to be included in the output\n", | ||
| " :param by: \"all\", \"domain\" or \"year\" the output to return.\n", | ||
| " :param minline: the minimum size of a line to be included in the output.\n", | ||
| " :return: [(year/domain, text)] or [text] depending on by\n", |
This comment has been minimized.
This comment has been minimized.
ruebot
Mar 5, 2019
Author
Member
@greebie the return comment here seems to be missing something. What should it read?
This comment has been minimized.
This comment has been minimized.
greebie
Mar 5, 2019
Collaborator
Maybe :return: [({year or domain}, textString)] if by is 'domain' or 'year', otherwise [textString]\n is more clear - it is also possible to include the type using :rtype: but I don't think that is too necessary.
greebie
approved these changes
Mar 5, 2019
ianmilligan1
reviewed
Mar 5, 2019
| "# characters to show per text file in output. Larger numbers will results in more\n", | ||
| "# text showing in output\n", | ||
| "# Characters to show per text file in output.\n", | ||
| "# Larger numbers will results in more text showing in output.\n", |
This comment has been minimized.
This comment has been minimized.
greebie
reviewed
Mar 5, 2019
| "# If you have a large file but want to sample the file more broadly, you\n", | ||
| "# can increase this value skip to every Nth line.\n", | ||
| "# If you have a large file but want to sample the file more broadly.\n", | ||
| "# You can increase this value skip to every Nth line.\n", | ||
| "RESULTS_STEP = 5\n", |
This comment has been minimized.
This comment has been minimized.
greebie
Mar 5, 2019
Collaborator
I wonder if RESULTS_STEP should be changed to 1 for the default example? This var (plus RESULTS_START & RESULTS_LIMIT) is used basically so that we do not potato browsers. Right now, the analysis takes every 5th line starting from 0 and going to 2500, for a total of 500 lines. Alternately, we could keep RESULTS_STEP the same, and increase RESULTS_LIMIT to get more results.
greebie
reviewed
Mar 5, 2019
| "def write_output (stdout, results):\n", | ||
| " \"\"\" Writes results to file.\n", | ||
| " \n", | ||
| " :param stdout: Filepath for file\n", | ||
| " :param results: A list of results.\n", | ||
| " :return: Nothing\"\"\"\n", | ||
| " :return: Nothing\n", |
This comment has been minimized.
This comment has been minimized.
greebie
reviewed
Mar 5, 2019
| "mapping = {x[0]: x[1] for x in graph.nodes('label')}\n", | ||
| "\n", | ||
| "# Use Archive Unleashed Clouds Positions (saves on load time)\n", | ||
| "# Use Archive Unleashed Clouds Positions (saves on load time).\n", |
This comment has been minimized.
This comment has been minimized.
greebie
reviewed
Mar 5, 2019
| "mapping = {x[0]: x[1] for x in neigh.nodes('label')}\n", | ||
| "\n", | ||
| "# Use Archive Unleashed Clouds Positions (saves on load time)\n", | ||
| "# Use Archive Unleashed Clouds Positions (saves on load time).\n", |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
If we're good with the review commit, I'll move on to the other notebook. Then get the other dataset in other PR. |
greebie
approved these changes
Mar 5, 2019
This comment has been minimized.
This comment has been minimized.
|
changes lgtm. |
ruebot
marked this pull request as ready for review
Mar 5, 2019
This comment has been minimized.
This comment has been minimized.
|
Both notebooks are added now. Let me know if there are any issues. If there are none, commit message should just be: Title: |
ruebot commentedMar 5, 2019
•
edited
Close read of text and comments. I'll start with the example notebook, and move on to the other.