Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Incorrect docs link for NDArray #12829

Closed
thomelane opened this issue Oct 15, 2018 · 11 comments
Closed

Incorrect docs link for NDArray #12829

thomelane opened this issue Oct 15, 2018 · 11 comments

Comments

@thomelane
Copy link
Contributor

All links for NDArray in the Python documentation link to mxnet.optimizer.NDArray instead of mxnet.ndarray.NDArray. Could add confusion for users, since this is the wrong reference to NDArray. And it's only technically possible because NDArray is imported in optimizer.py.

@thomelane
Copy link
Contributor Author

@mxnet-label-bot [Website, Doc]

@lanking520
Copy link
Member

@aaronmarkham can you take over from here? :)

@aaronmarkham
Copy link
Contributor

This seems related to this other issue (why can't Sphinx access the shorthand function calls):
#12318

Looking at the code section, it's clear that the erroneous links come from Sphinx's autodoc plugin. It is going through, importing things and checking the functions.
https://github.com/apache/incubator-mxnet/blob/master/python/mxnet/ndarray/utils.py#L235-L238
If it's linking to optimizer then maybe that's because the context that it is in at that point.

I get the feeling that init / relative import stuff is messed up. Help?

@aaronmarkham
Copy link
Contributor

@lanking520 noted that everything is fine in v1.3.0.
I compared the docs build process and note that the optimizer folder is new in master where as in 1.3.0 it was just a optimizer.py file.
We're seeing a bunch of errors in Sphinx:

/home/ubuntu/incubator-mxnet/python/mxnet/autograd.py:docstring of mxnet.autograd.mark_variables:None: WARNING: more than one target found for cross-reference u'NDArray': mxnet.optimizer.NDArray, mxnet.ndarray.NDArray
/home/ubuntu/incubator-mxnet/python/mxnet/autograd.py:docstring of mxnet.autograd.mark_variables:None: WARNING: more than one target found for cross-reference u'NDArray': mxnet.optimizer.NDArray, mxnet.ndarray.NDArray
/home/ubuntu/incubator-mxnet/python/mxnet/autograd.py:docstring of mxnet.autograd.backward:None: WARNING: more than one target found for cross-reference u'NDArray': mxnet.optimizer.NDArray, mxnet.ndarray.NDArray
/home/ubuntu/incubator-mxnet/python/mxnet/autograd.py:docstring of mxnet.autograd.backward:None: WARNING: more than one target found for cross-reference u'NDArray': mxnet.optimizer.NDArray, mxnet.ndarray.NDArray

@lanking520
Copy link
Member

After checking with @aaronmarkham The issue is introduced in this PR. There are a bunch of more issues that are cross-referenced...

@leezu
Copy link
Contributor

leezu commented Oct 20, 2018

This may be due to the python modules not correctly specifying __all__. The members of __all__ are the ones exported by a Python module. If __all__ is not defined

#12365 splits mx.optimizer into mx.optimizer and mx.optimizer.contrib. It introduced the __all__ statement. See https://github.com/apache/incubator-mxnet/pull/12365/files#diff-ee303d7550561edd63bea9a90d2f7106R24 https://github.com/apache/incubator-mxnet/pull/12365/files#diff-4b133b8d5dae8c5b7f6c8a872a244b3aR27 and https://github.com/apache/incubator-mxnet/pull/12365/files#diff-0c893416e9e93fbd94dfaa9fa6c13d67R34
Looking at the last of the 3 links, NDArray is wrongly part of __all__. I'll open a PR to remove it. This may also fix this issue.

@leezu
Copy link
Contributor

leezu commented Oct 22, 2018

Fixed by #12886

@lanking520
Copy link
Member

@leezu thanks for your diagnosis and fix on the potential issues. @aaronmarkham could you please try again to build based on Master to see if the issue is reproducible?

@leezu
Copy link
Contributor

leezu commented Oct 22, 2018

@lanking520 @aaronmarkham No need to rebuild based on master. CI did that already. The docs of CI's build are at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-12886/2/api/python/ndarray/ndarray.html#mxnet.ndarray.save and as far as I can tell don't contain the issue described by @thomelane

@aaronmarkham
Copy link
Contributor

@leezu Thanks for the fix! I checked on prod, and the cross-linking for NDArray looks correct now. I'm going to dig around in the logs and see if there are similar issues, but I think we can close this issue now.

@thomelane
Copy link
Contributor Author

thanks everyone for the quick fix! it's looking good from my side, so case closed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants