-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
PR: Edit tutorial to update it, fix errors, copyedit and clarify #7643
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks @CAM-Gerlach!
@ccordoba12 Thanks! I'll go ahead with the follow-on improvements to further enhance its appearance and improve maintainability and consistency by conforming it to our docs style guide, in a similar manner to spyder-ide/spyder-docs#13 , but that's much less critical in terms of user impact so its not vital that it makes it into 3.3.1 (though it'd be nice, if the PR is final and the milestone is still open). |
Sure, please do it. Worst case, it'll land in 3.3.2 |
That's the idea! |
@@ -179,31 +165,31 @@ Inspecting objects defined in the console | |||
Print "Hello World" and return None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a full stop after None
(as this has been added to the docstring in line 38 above), i.e. change return None
to return None.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @fangohr for chiming in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Professor @fangohr , thanks! Good catch; once you're done reviewing it I can submit a PR and fix everything you find. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, just FYI #7659 has the up to date version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed three different instances as requested. Thanks!
Editor. This feature is very useful, and should be used | ||
routinely. Do try it now if auto-completion is new to you. | ||
- ``Tab``\* auto-completes commands, function names, variable | ||
names, and methods in the console and the Editor. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Editor should be lower case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at other examples below, "editor" there is also spelled "Editor", i.e. upper case. From a purely language (english) point of view, there is no reason for spelling editor with an uppercase E. One could take the view that the "Editor" is an important part of Spyder, and for that reason it should be written with capital "E". In that case, should we do it consistently, and also capitalise "Console"?
Again, nothing major, but I wanted to point out the slight inconsistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, should we do it consistently, and also capitalise "Console"?
This is a very good point, and exactly what I've done for the tutorial in the followup #7659 as well as the rest of our docs. As I've specified in our docs style guide (to be formally published in full shortly), the Editor, the Console, the Variable Explorer and the Help pane are proper nouns, i.e. the proper names of core modules of Spyder, and thus they should be capitalized and are also in a different font/style (:guilabel:`Editor`
) to indicate they are the names of key GUI elements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, thanks.
cursor to the next cell (menu entry ``Run > Run cell and | ||
advance``). | ||
|
||
Cells are useful to execute a large file/code segment in smaller | ||
units. (It is a little bit like a cell in an IPython notebook, in | ||
that chunks of code can be run independently.) | ||
that chunks of code can be run independently). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may be a matter of taste (and is not important), but I'd argue that the previous version ".)" is better than the new one ")." as the parenthesis enclose the extra sentence and the full stop below to the sentence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would argue that this
Cells are useful to execute a large file/code segment in smaller
units (it is a little bit like a cell in an IPython notebook, in
that chunks of code can be run independently).
OR
Cells are useful to execute a large file/code segment in smaller
units; it is a little bit like a cell in an IPython notebook, in
that chunks of code can be run independently.
Is better :-p
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd argue that the previous version ".)" is better than the new one ")."
You're indeed correct at least per AP style (which is what we nominally follow per our style guide) and it was I who was mistaken—for a full sentence inside parenthesis, the period/full stop goes inside instead of outside as normal (I've generally avoided the former in my writing to begin with as a matter of preference, so I wasn't as familiar with that exception as I should have been).
Cells are useful to execute a large file/code segment in units (it is a little bit like a cell in an IPython notebook, in that chunks of code can be run independently).
The problem with this syntax is that its a bit of a sentence splice there to just use a parenthesis instead of a "stronger" parenthetical (like the semicolon you used below), since both are complete sentences.
Cells are useful to execute a large file/code segment in smaller units; it is a little bit like a cell in an IPython notebook, in that chunks of code can be run independently.
This works much better. With some further refinements for flow, plural/singular discontinuity, and conciseness, I propose the following:
Cells are useful for breaking large files or long blocks of code into
more manageable chunks—like those in an IPython notebook,
each cell can be run independently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ended up just splitting with a period instead of a semicolon or a em dash; otherwise implemented the above. Let me know what you think.
debugger (ipdb) if the IPython console is active. After doing that, the | ||
Editor pane will highlight the line that is about to be executed, and the | ||
Activating the debug mode (with the ``Debug > Debug`` menu option or ``Ctrl+F5) | ||
the IPython debugger ``ipdb``. The Editor pane will then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"starts" is missing before "the IPython debugger".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, silly mistake, and its already fixed in #7659 .
modify ``x`` by typing ``x=10`` in the debugger prompt. You see x | ||
changing in the Variable Explorer. You should also see ``x`` changing | ||
Keep pressing the ``Step`` button to execute the next lines. Then, | ||
modify ``x`` by typing ``x = 10`` in the debugger prompt. You should see x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"x" at the end of the line needs to have double backpacks on either side for correct rendering: "x
"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good eye; will do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed!
Thank you for updating and improving the tutorial in so many ways, including content, english language and style. |
@fangohr Sure, of course! Sorry we didn't consult your input earlier in the process; it was just extremely rushed over the course of about 24 hours from when we started to when it needed to go in so as to make it into Spyder 3.3.1. Thanks so much and glad you approve! I'm really honored! |
Pull Request Checklist
Description of Changes
@ccordoba12 Edits the tutorial to update or eliminate large portions that were obsolete or out of date, as well as fix a number of typos and grammar errors, improve the overall writing quality and enhance the clarity of the various descriptions. Formatting, style-guide, and any substantial additions were deferred to another PR, to best aid a quick review, and I tried to be rather conservative with the changes overall (though I probably could have been stricter still). I loaded up the tutorial inside of Spyder and looked though all of it visually, and it all seemed to look good, and all the internal and external links I tested worked. However, I couldn't do a full Sphinx build due to its custom nature, at least for the moment.
Issue(s) Resolved
Fixes #7641