Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

python 2 compatibility for class inheritance #5

Closed
guillaumeeb opened this issue Apr 17, 2018 · 5 comments

Comments

@guillaumeeb
Copy link

@guillaumeeb guillaumeeb commented Apr 17, 2018

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 dask/dask-jobqueue#35.

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

@Chilipp

This comment has been minimized.

Copy link
Owner

@Chilipp Chilipp commented Apr 17, 2018

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

This comment has been minimized.

Copy link
Author

@guillaumeeb guillaumeeb commented Apr 17, 2018

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

@guillaumeeb

This comment has been minimized.

Copy link
Author

@guillaumeeb guillaumeeb commented Apr 17, 2018

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

This comment has been minimized.

Copy link
Owner

@Chilipp Chilipp commented Apr 17, 2018

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

This comment has been minimized.

Copy link
Contributor

@lesteve lesteve commented Apr 18, 2018

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.

Chilipp added a commit that referenced this issue Apr 18, 2018
Should finally solve #5 and
#2
Chilipp added a commit that referenced this issue Apr 18, 2018
Should finally solve #5 and
#2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.