Examples are not pep-8 compliant
Opened this issue · 8 comments
Some of them are not even compliant with the stated standard of:
from __future__ import print_function, divisionSince these are used for training and education I feel that it is important to set a good example.
It looks to me as though the main objection to the example programs is the use of "from visual import *", which in general is considered bad usage for many good reasons. However, this form of import is highly appropriate in the VPython context, where a very large fraction of the users are complete novices with no prior programming experience, a large number of whom are science and engineering students in an intro course that uses the Matter & Interactions curriculum. For these users it is crucial to reduce to the barest minimum any element of programming extraneous to the challenging task of computational modeling of physical systems. This includes not having to remember always to say vp.vector and vp.color and vp.sphere, etc. (As it says on the first page of the VPython help, experienced programmers will presumably wish to avoid "from visual import *".) Moreover, VPython programs are typically very short, without imports of other modules. Another example: I see in your revision of drape.py that you've changed m to M and g to G. That may be good PEP8 style, but it's unacceptable in this physics program, where by ancient physics consensus g is 9.8, G is 6.7e-11, and m and M are used for different masses in many situations. I do agree that the example programs should have the future line; its absence is an artifact from the past (many of the example programs are 15 years old).
While I agree with the point about g vs G, etc., but do think that failure to use namespaces is a recipe for problems moving on, especially since visual redefines a number of built-in components - I specifically had problems with sum not working on some of the example programs. If you are agreeable I may continue making the examples PEP8 style - possibly with an override on physical constants in the fork.
I don't see how you could have had a problem with sum if you don't use "from visual import *", nor do I understand how an example program could fail due to this. If you do use this form, all of numpy is imported, as well as all of math. Experienced programmers are counseled on the first page of the VPython documentation about these issues. I will maintain the examples as they are, for the reasons I've given. Note too that the documentation is written under the assumption of using "from visual import *". We do understand that novices have much to learn to go beyond novice status, and one of the many things they need to learn is about namespace issues. But we want to get them off the ground quickly.
My mechanism for addressing the from visual import * is delete the line and replace with import visual as vp and then run pylint to get all of the places where I need to add vp. - since sum is a built in it doesn't give an error report but on crytsal.py it freezes on the first update.
Personally I think that there is an element of "we don't wish to put new drivers off of driving by making them use seat belts" to the namespace argument and that making it clear what comes from visual, as opposed to python, would be a good thing.
Your seat belt analogy is not inappropriate, but there are other possible analogies, such as the fact that it is easier to learn to drive in a car with automatic transmission, even if later you become a professional race car driver, or that training wheels are useful to a kid just learning to ride a bike. The vast majority of VPython users are not learning computer science but are being introduced to computational modeling of physical systems, a quite challenging task which requires that there be as few distractions as possible. There is time later for these people to write larger programs with attention paid to namespace issues, subtleties of the stability of numerical solutions, etc.
Can I renew my suggestion of reverting the current pushes and instead putting a "name spaced" selection of the examples in a subdirectory for any users that are interested?
Sure, that would be fine. I still don't feel very confident of my GitHub skills, so the easiest thing would be to give me a zip file containing all of your modified files, which I would add to the repository in its own folder. Thanks.
Will do - it will be a few days as I have a lot on at the moment.