fix 'Alias for field number X' problem with NamedTuples

[fohrloop]

Fixes #257

[pitrou]

Does it work properly with #257 (comment) ?

[fohrloop]

Tested with code from this comment, using

.. autoclass:: mypkg.Foo
    :members:

that outputs:

does that answer to the question?

Edit: Added also the methods to the screenshot.

[pitrou]

It does, thank you!

[fohrloop]

As a side note, it might be cleaner to have all "should_skip_member" -type of logic under one method. Right now there is

                not name.startswith("_") # <----- this
                and not self._should_skip_member(name, self._cls)
                and ( #  <--------- this
                    func is None
                    or isinstance(func, (property, cached_property))
                    or inspect.isdatadescriptor(func)
                )
                and self._is_show_member(name) 

...


    def _is_show_member(self, name):
        if self.show_inherited_members:
            return True  # show all class members
        if name not in self._cls.__dict__: # <------- and perhaps this?
            return False  
        return True

which could be moved under one place.

[rossbar]

Thanks @fohrloop ! I took the liberty of pushing up a test that fails on main and passes after this change. This does indeed handle the duplication of the named fields mentioned in #257 .

PS; I was surprised to learn that there is no way to do isinstance checks with named tuples - you learn something new everyday!

[fohrloop]

Thanks @rossbar for the test! Let me know if there's need for additional tests. For example, could it be tested that namedtuple with some custom docs in the params like this works as expected:

class SomeClass(NamedTuple):
    """A docstring.
    """
    
    foo: str = 'text'
    """This is a docstring of foo 
    """

It was tested manually, but I don't know if it would be beneficial to have such automated test, too. Not sure how to implement that one, though.

Is there something else that should be done here? Does the code need refactoring..? Or something else?

[rossbar]

For example, could it be tested that namedtuple with some custom docs in the params like this works as expected

That's a good idea, but shouldn't be a blocker for this PR. I'll try to add one today if I have time but don't wait on me - if someone pushes the green button in the meantime that's a positive outcome!

Is there something else that should be done here? Does the code need refactoring..? Or something else?

No other action necessary IMO - this is good to go. The only reason I didn't merge it myself is because I pushed up some code, so someone should have an opportunity to take a look at that. If it's not in by this evening I'll merge it!

[rossbar]

A day late, but I think I managed to design a test that captures your last comment @fohrloop - take a look and LMK what you think. If it looks correct to you I'll go ahead and merge!

[fohrloop]

@rossbar awesome, thanks! It seems to test well that the MyFooWithParams.bar and MyFooWithParams.baz are included in the documentation. Would it make sense to test also that the content of the docstring is there? Something like

assert sds["Parameters"][0].desc[0] == "The bar attribute"

On the side note I don't fully undestand why the .bar and .baz are listed under "Parameters" and not "Attributes".

[rossbar]

On the side note I don't fully undestand why the .bar and .baz are listed under "Parameters" and not "Attributes".

This is because they are included under the Parameters heading in the class docstring. Without that, they wouldn't be there at all (i.e. len(sds["Parameters"]) == 0 for the previous test case).

I think all the bases are now covered, so I'll go ahead and get this in, but if there are other corner-cases related to namedtuple/dataclasses, please don't hesitate to continue fleshing them out!

[fohrloop]

myst_parser compatibility

I just did some tests with this using numpydoc 1.7.0rc0.dev0 installed from latest master (commit 46f532a):

python -m pip install https://github.com/numpy/numpydoc/archive/main.zip

It turns out that if you use numpydoc with myst_parser, numpydoc must be listed before myst_parser in the conf.py;

This works:

#conf.py

extensions = [
    "numpydoc",
    "myst_parser",
    ...
]

This will not work:

#conf.py

extensions = [
    "myst_parser",
    "numpydoc",
    ...
]

(just commenting this here so that if someone has problems with this they know what to look at)

[rossbar]

Thanks for bringing this up @fohrloop - this is worth it's own, separate issue here (and potentially with myst_parser) so that any config collisions between extensions can be resolved. Feel free to do so, else I will cp/paste the above to a new issue later.

[fohrloop]

Yeah I'm not sure if this is an "issue" or just a feature. Not sure what myst_parser does in between, but I'd guess the document tree (or what it is called) is converted somehow and numpydoc does not understand it anymore? If this is solvable in a way that numpydoc and myst_parser could be placed in either order, that's great, but I can myself live with that ordering. Just hoping that there will be no other issue which would demand the opposite order 😁🤞

And it's likely that this could/should be solved in myst_parser side as it's the one first in the list when there are problems.