Chilipp / docrep

A Python Module for intelligent reuse of docstrings
Apache License 2.0
30 stars 4 forks source link

python 2 compatibility for class inheritance #5

Closed guillaumeeb closed 6 years ago

guillaumeeb commented 6 years ago

We are using docrep in https://github.com/dask/dask-jobqueue project to avoid docstring duplication. However, we've got a problem with python 2, see https://github.com/dask/dask-jobqueue/issues/35.

I see in #2 that you may have solutions for this?

Chilipp commented 6 years ago

Hi! Yes, it is indeed the problem that you cannot modify the docstring of a class in python2 after it has been created. Therefore you would need something like

class PBSCluster(JobQueueCluster):
    __doc__ = d.with_indents("""
        Docstring from PBSCluster

        Parameters
        ----------
        %(Params2use)s
        """, 8)

Does this help you?

guillaumeeb commented 6 years ago

Thanks, I will try that and I'll let you know!

guillaumeeb commented 6 years ago

Yep, that seems to fix it!

Only downside is it kinds of break the Python docstrings class standard use, but it does the job.

Thanks again.

Chilipp commented 6 years ago

Sure I know, I also do not like this solution so much in my own projects. Therefore, I by myself actually use the following solutions (although the second is kind of advanced):

  1. Most of the time, I use the __init__ method for the "Parameters" section (sphinx can handle that if you for example set autoclass_content = 'both' in the conf.py for the autodoc extension).

    class PBSCluster(JobQueueCluster):
       """Docstring of PBSCluster"""
    
       @d.dedent
       def __init__(self, *args, **kwargs):
           """
           Parameters
           ----------
           %(Params2use)s
           """
  2. Where I however use it generally for all subclasses, I use it in a metaclass (I do that for example for my Formatoption class)

    import six
    from abc import ABCMeta
    
    class PBSClusterMeta(ABCMeta):
    
       def __new__(cls, clsname, bases, dct):
           """Assign an automatic documentation to the class"""
           dct['__doc__'] = d.with_indent(dct.get('__doc__'), 4)
           return super(PBSClusterMeta, cls).__new__(cls, clsname, bases, dct)
    
    @six.add_metaclass(PBSClusterMeta)
    class PBSCluster(JobQueueCluster):
       """Docstring from PBSCluster
    
       Parameters
       ----------
       %(Params2use)s
       """
lesteve commented 6 years ago

Thanks a lot for the details!

I am probably very biased (my Python experience is mostly in the Scientific Python ecosystem), but the convention I have seen most of the time is to document constructor parameters in the class docstring as mentioned in the numpydoc guide:

The constructor (__init__) should also be documented here, the Parameters section of the docstring details the constructors parameters.

To sum up option 1. does not really follow widely used conventions in the Scientific Python ecosystem. About option 2. I'd rather avoid metaclasses because of the advanced aspect you mentioned.

Now the question is whether there is a clever way to support Python 2, maybe using option 2. inside docrep (rather than having docrep users do it themselves) would be a possible solution. Full disclosure: I am not a metaclass expert ...

I completely understand if supporting Python 2 is not one of your top priority (especially given that a lot of packages, for example numpy, are going to be "Python 3 only" starting in January 2019). Maybe a reasonable stop gap solution that involves not too much work would be a dump try/catch. This way for Python 2 the docstring is not complete but at least you don't get any error. And then you can add a gotcha in your documentation rewriting class docstrings in Python 2 with the known work-arounds.