Language fixes #750
Merged
Language fixes #750
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I find this unclear (and it should be according not accordingly): - **Rigid Body**: The object will behave accordingly to its shape. Do you mean: - **Rigid Body**: The object will behave according to its shape's properties. It seems confusing to me that Blender has Sensors and Actuators and so does morse but the morse ones are different. Pity the morse ones aren't called, say BotSensor(s) and BotActuator(s), but I guess it is too late now:-) I'm surprised you haven't added an option to Blender to select a whole robot since it is tricky, but perhaps that isn't possible.
I removed spurious '\'s from one of the code snippets. Using '\'s is poor style and unnecessary. I also changed the last example's def name(): to def name(self). Or did you mean @staticmethod def name(): ?
I find this unclear. all the objects ending with the empty, and press :kbd:`Shift+M`).
Don't like the name semanticC for the camera. Would prefer camera if there's just one or catEyeCamera if there's more than one. I don't understand the word "Sick"; should it be "Stick"?
Personally, I'd call the cameras leftCamera and rightCamera (or cameraLeft and cameraRight). Maybe "Sick" should be written "SICK"?
The code examples use tabs rather than 4 spaces contradicting PEP8.
In some files you write Menu >> Option and in others Menu > Option.
Personally I'd write Menu→Option.
Also, in some files you write :kbd:`Ctrl+a` and in others
:kbd:`Ctrl + a`; personally, I'd always use the former.
Also, the Tab and Enter keys are always capitalized.
In this file you write with Morse() as sumu: elsewhere you write
with Morse() as sim: which I prefer.
Shouldn't really encourage people to use % formatting. I'd change
pose_received to:
def pose_received(pose):
print("The Ranger is currently at {}".format(pose))
In most of the API you use dicts or **kwargs for parameters so at least
people know what they are setting. But for Actuator.add_data() it is
just *args which seems a pity. Also the type being specified by a str
(e.g., 'float') rather than by a type object seems strange.
I don't know what is meant by "for every lecture of the sensor".
You mention the use of avconv but don't show an apt-get line for obtaining it (or for obtaining the necessary codecs).
For rqt_plot the x and y arguments are , separated but the z is space separated which seems odd to me? I changed 'paste the following code' to 'type the following code' because if you paste a lump of code like this into the REPL it won't work unless you do it line-by-line (plus one block for the function).
Around line 190+ I added some information which appeared to be missing, so worth checking if I got it right:-)
Is this true? - The first line tells MORSE that this is a builder script. Does MORSE actually read the text of the .py file? Or should it be something like: - The first line imports the entire **Builder API** for use in the script. (Of course, import * is usually poor practice.)
I never recommend using pip or pip3; always python3 -m pip so that you always use the correct Python.
Got rid of lame joke.
|
Reviewed-by: Matthieu Herrb |
crey0
pushed a commit
to crey0/morse
that referenced
this pull request
Jan 16, 2018
crey0
pushed a commit
to crey0/morse
that referenced
this pull request
Jan 16, 2018
severin-lemaignan
pushed a commit
that referenced
this pull request
Jan 16, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a first raft of changes. I'll try to do more next week.