Confusing documentation for python3-ev3dev v2

[ndward]

The 13th principle of the Zen of Python: There should be one -- and preferably only one -- obvious way to do it.

Reminder: documentation for python3-ev3dev v2 can be found at https://python-ev3dev.readthedocs.io/en/ev3dev-stretch/index.html .

As I understand it, python3-ev3dev v2 contains all the same functions as python3-ev3dev v1 plus a number of new functions, most of which rare improved versions of the v1 functions, more in line with E3-G. Thus we have, for example, for single motors, on_for_degrees(speed, degrees, brake=True, block=True) which is similar to but superior to run_to_rel_pos(kwargs). And we have play_tone() to ultimately replace tone(). Of course we also have a number of great new commands such as the MoveSteering and MoveTank** commands which do not have equivalents in python3-ev3dev v1 but which do have equivalents in EV3-G.

I think the current documentation for v2 does not make it clear which are the new (preferred) functions and properties and which are the old (soon-to-be-deprecated) functions and properties. And the presence of both the old and new functions mixed up together on the same pages makes the pages very long and hard to navigate. For example, the v2 documentation on motors https://python-ev3dev.readthedocs.io/en/ev3dev-stretch/motors.html is more than 4500 words, even though the description of each function is very concise.

I suggest that it should be made very clear which are the functions and properties that are obsolescent now that superior new versions are available. I think it might even be preferable to move all the deprecated functions and properties to a separate page or at least a clearly-marked separate section within the same page. Until this is done users will be very confused about which functions they should be using and about which properties correspond to which functions. Take for example the documentation for STOP_ACTION_BRAKE = 'brake': Power will be removed from the motor and a passive electrical load will be placed on the motor. This is usually done by shorting the motor terminals together. This load will absorb the energy from the rotation of the motors and cause the motor to stop more quickly than coasting.

Is it obvious from this whether this is applicable to the older 'run' motor commands or the new 'on' ones or both? I don't think so.

[WasabiFan]

@ddemidov @dwalton76 What are your thoughts on this? I generally agree that most people will and should use the new on_* methods rather than the manual run_* variations, and further I expect that practically no-one will be directly poking the driver with speed_sp, command, etc. They're there because that was originally the primary way you could use the API surface, but really don't need to be anymore. That being said, they certainly need to be accessible somewhere. If we're going to change it, I think we should do it now rather than having to bump the major version later.

If we do want to change the structure, it seems to me that the best way to do it would be to put the driver properties on a new object which is available under a field on each device object, e.g. native or driver. So you'd do my_motor.native.speed_sp. That would let us move all the documentation for those properties to a page specifically for it.

Alternately, we could take the easier route which doesn't introduce any breaking changes, and re-order the properties so that on_* for motors (and the various value properties for sensors) are first and everything else is after.

[ddemidov]

How about just creating a subsection in the Motor docs, named 'Low level API' and containing native driver commands? This way we keep our current API and let people know what is functions are designed to be used by the user.

[WasabiFan]

I like the idea, but I'm not sure how to do it with the autoclass directive that we use. Do you have any idea? My recollection is that it's quite unconfigurable.

[WasabiFan]

We might be able to hack something together using http://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html#event-autodoc-skip-member

[ddemidov]

The following code in motors.rst

Tacho Motor (``Motor``)
~~~~~~~~~~~~~~~~~~~~~~~

.. autoclass:: Motor

   .. rubric:: Attributes

   .. autoattribute:: COMMAND_RUN_TO_ABS_POS
   .. autoattribute:: COMMAND_RUN_TO_REL_POS

   .. rubric:: Methods

   .. automethod:: on_for_seconds
   .. automethod:: on_for_rotations

   .. rubric:: Low level methods

   .. autoattribute:: speed_sp
   .. automethod:: run_forever

Gives me something like:

This is more work than just using :members:, but gives more control over contents.

[WasabiFan]

Yeah, that seems reasonable; we'll just have to be careful that we always add new things to the docs.

[ndward]

Another good example of confusing and superfluous documentation: https://python-ev3dev.readthedocs.io/en/ev3dev-stretch/sensors.html#ev3dev2.sensor.lego.UltrasonicSensor.distance_centimeters_continuous Ultrasonic distance_centimeters_continuous and distance_inches_continuous have notes that they should not be used because there are already properties that do exactly the same (and which have less verbose names). I can't see any reason why users should be told about these properties, only to then be told that they should not be used, nor do I understand why these properties were added in the first place.