project8 / dripline-python

python implementation of project8/dripline
Other
2 stars 0 forks source link

better fancy_doc #27

Open wcpettus opened 6 years ago

wcpettus commented 6 years ago

In the words of Ben, "fancy_doc is an ugly hack."

One way this has come back to bite us is that it breaks super - fancy_doc creates a new inherited class at run time, so that the method-resolution order is wonky:

>>> dragonfly.implementations.PrologixProvider.__mro__
(<class 'dragonfly.implementations.prologix_provider.PrologixProvider'>,
 <class 'dragonfly.implementations.prologix_provider.PrologixProvider'>,
 <class 'dragonfly.implementations.repeater_provider.RepeaterProvider'>,
 <class 'dragonfly.implementations.repeater_provider.RepeaterProvider'>,
 <class 'dripline.core.provider.Provider'>,
 <class 'dripline.core.provider.Provider'>,
 <class 'dripline.core.endpoint.Endpoint'>,
 <class 'dripline.core.endpoint.Endpoint'>,
 <type 'object'>)

A level-1 better implementation would be to add a utility which is run in Endpoint.__init__ which will fix the docstrings for us. This would remove every @fancy_doc call from the code and fancy_doc import, which would be nice; it would also remove duplicate instances in the mro tree.

The level-1 fix will break whenever RTD does the promised upgrade where it doesn't import but builds straight from the docstrings. We'd need an RTD fix then.

laroque commented 6 years ago

To clarify the above slightly. The level-1 better implementation is to implement a utility function (probably in fancy_doc.py) which is executed in dripline/init.py after everything has been imported (and also in dragonfly/init.py or anything else that builds on dripline-python) which implements the existing doc-string logic for every class which inherits from Endpoint. This would mean developers don't need to remember to decorate with @fancy_doc (it would also make the entire thing non-optional).

Additionally, instead of creating a new class with the modified doc strings, we could (I think) modify the existing docstrings (which are class variables) at runtime. This should mean that we don't double up the number of classes in the MRO, but simply modify the attribute of the class before anything using the library has a chance to get at them.

If we want the fancy_doc thing to be optional, we could instead place the utility function in a mix-in class and have Endpoint inherit from it. The init.py file function would then call the fancy_doc function from the mix-in on each class, and any class wanting to avoid it would be able to override the function with a function which does nothing (or custom version with other logic).