Documentation/Coding style issues

severin-lemaignan commented 7 years ago

[issues collected from Mark Summerfield PR]

[ ] #739 The term classpath may confuse Java programmers; why isn't it called robotpath?
[ ] #737 Personally, I'd change the my_repr variable's name to output or data throughout.
[ ] #737 Also, in many places I'd have replaced "specific" with "custom".
[ ] You could consider adopting the new typing module's syntax, e.g., _type_name = typing.List[int, int, int], etc.
[ ] #737 The - signs on lines 339, 340, 342, and 343 look wrong to me. Personally I never leave whitespace after a unary operator.
[ ] #736 Line 174 where does mathutils come from?
[ ] #736 Line 253 - why isn't it: image = self.get_raw_image() # i.e., ending with ()?
[ ] #736 I really dislike the add_data()/add_property()/add_level()/local_data API which I find very un-Pythonic.
[ ] #736 It seems like you want people to save the new components they create inside MORSE's own directory tree. So what happens if they create a component that has the same name as a new built-in component and they update MORSE?
[ ] #736 I don't recognize the word "revolute" on line 325. Should it be "revolving" or "bendable" or "movable"?
[ ] #736 On line 382 you have Visible_arc but all the other params are lowercase, so is this correct?
[ ] You might like to consider using the flake8 tool to keep the code all uniform.
[ ] #730 Here you give instructions for tcsh and bash; elsewhere for csh and bash. Personally I think you should always give bash first and if you give the others then consistently csh and/or tcsh afterwards.
[ ] #726 Using * imports is bad style. You should show how to use morse without using them.
[x] #726 In bash, if you type "morse run my" and then tab-complete you'll get "morse run mysim/" which will produce an error. Surely, it could be smart enough to look for "mysim/" and if it doesn't find it fall back to trying "mysim"?
[ ] #726 Most of the English seems British, but then you use the American meter rather than British (French!) metre.
[ ] #726 On line 231 I don't understand where the atrv object comes from; why isn't it robot.rotate(...)?
[ ] #726 Clearly the auto-naming is useful for beginners, but surely you should advise people to explicitly name?
[ ] #726 Sometimes morse is written MORSE and other times Morse -- you ought to choose one and keep to it throughout:-)

PierrickKoch commented 7 years ago

For the spelling issue #726, I'd vote for using "MORSE" full-caps as the name. The only place we should use "Morse" is in Python class names, and "morse" in Python instances.

nicolaje commented 6 years ago

The trailing slash auto-removal was implemented in PR #754 We should be able to clear that issue easily and quickly.

morse-simulator / morse

Documentation/Coding style issues #744