Fix commands to make doc consistent #14243

ChaiBapchya · 2019-02-23T22:43:33Z

aaronmarkham

Minor fix requested in the review... otherwise looks good. Thanks for these fixes!
I'd update your overview text to say "fixes #xxxxxxx" so that it should trigger an auto-close when this PR is merged. I'm not sure if "refer to" works the same way.

aaronmarkham · 2019-02-25T22:47:36Z

src/operator/swapaxis.cc

-  swapaxes(x, 0, 1) = [[ 1],
-                       [ 2],
-                       [ 3]]
+  >>> x = mx.nd.array([[1, 2, 3]]) //(1,3) array


This comment breaks things. I'd just get rid of it... or put the comment on it's own line.

aaronmarkham · 2019-02-25T22:47:51Z

src/operator/swapaxis.cc

-        [ 2, 3]],
-       [[ 4, 5],
-        [ 6, 7]]]  // (2,2,2) array
+  >>> x = mx.nd.array([[[ 0, 1],[ 2, 3]],[[ 4, 5],[ 6, 7]]]) // (2,2,2) array


Comment breaks things.

aaronmarkham · 2019-02-26T00:39:43Z

BTW, one reason I can a sort of see why there's this abstraction from Python-specific code, is that this docstrings are pulled into the other API's docs. I'm not saying it's the best way, but it is what's going on. So if each of these sections are updated, and there are tons more in this state, we'll see very pythonic code examples in the R or Scala docs (not just these either). That could make things worse as opposed to these code examples being more of pseudo-code.

ChaiBapchya · 2019-02-26T20:22:20Z

@aaronmarkham Thanks for your review. Your point makes sense.

In that case, we need to close this PR and start another PR that changes the examples in Pythonic way to pseudocode

At the moment, there are few examples in Pythonic way, few in pseudo-code.

We need to settle on one.
@aaronmarkham
What do you think is better?
Should we start a discussion on dev for this? or it's too trivial for that?

aaronmarkham · 2019-02-26T21:23:08Z

I emailed the dev list about this issue yesterday. No response yet. I'm really not sure what the best way to proceed is.... I agree the examples should be executable, but things are complicated by how much is borrowed to support the docs for the other APIs. Let's see what dev@ says.

vandanavk · 2019-03-04T21:55:47Z

@mxnet-label-bot add [pr-work-in-progress]

aaronmarkham · 2019-03-06T19:19:12Z

@ChaiBapchya - here's Mu's response to my question though I'm not sure how it applies to this PR.
@mli Can you take a look at this PR and provide your suggestion?

eric-haibin-lin · 2019-03-06T20:15:12Z

src/operator/nn/concat.cc

-                          [ 5.,  5.],
-                          [ 6.,  6.],
-                          [ 7.,  7.],
-                          [ 8.,  8.]]


Have the community decided to remove all language-agnostic pseudo code example? Other language binding may loose the only available example

aaronmarkham · 2019-03-06T21:18:55Z

Let's take the concat operator example. Assume we leave in the pseudo-code as is.
Then we still have something that doesn't execute in the Python API docs. All I'm seeing is auto-summary outputs for ndarray.concat or ndarray.sparse.concat or symbol.concat which just dump whatever is in the concat.cc file's docstring.

So if we were to add the example in runnable python code, where would it go?

eric-haibin-lin · 2019-03-12T07:19:44Z

@aaronmarkham it should go to https://github.com/apache/incubator-mxnet/blob/master/python/mxnet/ndarray_doc.py
You can see examples for ndarray.Reshape and ndarray.elemwise_add.

aaronmarkham · 2019-03-12T14:32:55Z

Ok I asked Mu about that file on the dev list. I am beginning to understand, but I feel like there's a lot missing. Are you saying that 99% of the examples for ndarray do not exist because they're not in that file?
I'm not clear yet on when something goes there versus elsewhere and why there aren't any more xyz_doc.py files.

eric-haibin-lin · 2019-03-24T23:45:30Z

I think it's just a way to append python specific examples

piyushghai · 2019-04-09T00:33:33Z

@ChaiBapchya Can you address the review comments and rebase with the latest master branch ?

ChaiBapchya · 2019-04-09T01:04:50Z

@piyushghai I am still waiting for conclusive next steps. I am not sure if consensus was reached on dev list or the discussion here. Any idea how we can get clarity? Thanks.

eric-haibin-lin · 2019-04-09T04:26:23Z

I am suggesting that python specific examples should be added to https://github.com/apache/incubator-mxnet/blob/master/python/mxnet/ndarray_doc.py#L57
Is anything not clear?

This reverts commit 48f926f.

ChaiBapchya · 2019-04-10T09:17:12Z

@eric-haibin-lin Wanted to verify if the latest commit is correct? Also not sure if all the doc should go in this one file alone or some other separate file and is there any other file that needs to know to "refer to python/ndarray_doc? Just making sure I haven't missed it. Thanks.

aaronmarkham

Can you provide a preview link? I'm curious how this is looking and how to compare it to what's live now.

aaronmarkham · 2019-04-10T16:44:01Z

python/mxnet/ndarray_doc.py

+    <NDArray 2x2 @cpu(0)>
+    """
+
+class ReshapeLikeDoc(NDArrayDoc):


How are you deciding when to capitalize?

I was actually confused. Originally, there were 2 variants
Capitalized - StackDoc, BroadcastToDoc, ReshapeDoc
Non-capitalized - elemwise_addDoc

I am not sure if I understood the difference. Any help on those lines would be great. Thanks for pointing out.

ChaiBapchya · 2019-04-10T18:05:06Z

@aaronmarkham I'm sorry I'm unaware. Can you tell me where I can find preview links for docs? Thanks.

eric-haibin-lin

This is a good start. Preview at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-14243/4/api/python/ndarray/ndarray.html#mxnet.ndarray.Concat

python/mxnet/ndarray_doc.py

aaronmarkham · 2019-04-19T19:50:34Z

Restarted CI job (some MKL-DNN test failed, so let's see if it goes through this time...)

aaronmarkham · 2019-04-26T15:05:27Z

python/mxnet/ndarray_doc.py

+    <NDArray 1 @cpu(0)>
+    """
+
+class CastDoc(NDArrayDoc):


Why do you think that return type is gone now? Look at prod, then look a the preview....

Absolutely no idea why.

python/mxnet/ndarray_doc.py

aaronmarkham · 2019-04-26T15:13:07Z

python/mxnet/ndarray_doc.py

+    >>> y = mx.nd.array([[3,3],[4,4],[5,5]])
+    >>> z = mx.nd.array([[6,6], [7,7],[8,8]])
+
+    >>> mx.nd.concat(x,y,z,dim=0)


I'm not seeing this show up in the preview...

I do see it. Did you look at the right one?

http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-14243/7/api/python/ndarray/ndarray.html#mxnet.ndarray.Concat

The preview link is versioned, as mentioned in the pull request checklist:

Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html

Why does the previous API render the interpreter prompt, but this one does not? It looks close, but I think they're different because it is missing these:
>>> y = mx.nd.array([[3,3],[4,4],[5,5]])

Any idea why?

It looks close, but I think they're different because it is missing these:
>>> y = mx.nd.array([[3,3],[4,4],[5,5]])

I did find this. What do you think is missing????

Ya, I guess he was looking at some other preview doc.

aaronmarkham · 2019-04-26T15:15:07Z

python/mxnet/ndarray_doc.py

+
+    Return the elements, either from x or y, depending on the condition.
+
+    >>> x = mx.nd.array([[1, 2], [3, 4]])


This doesn't render right.

aaronmarkham · 2019-04-26T15:19:07Z

There are some rendering issues - likely just needs some extra line returns. Also, I think that because there are operators that use opname and Opname that seem to be similar, the docs functions should map exactly. For example, concat and Concat would have concatDoc and ConcatDoc.

aaronmarkham

Latest preview seems ok. http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-14243/10/api/python/index.html

karan6181 · 2019-05-21T21:25:33Z

@ChaiBapchya Thanks for your contribution!
@eric-haibin-lin Could you please review it again? Thanks!

addressed

ChaiBapchya · 2019-11-05T01:37:23Z

Since the launch of the new website, we don't depend on this file anymore. Hence closing this as stale PR

fix commands to make doc consistent

48f926f

aaronmarkham suggested changes Feb 25, 2019

View reviewed changes

aaronmarkham added the Doc label Feb 25, 2019

marcoabreu added the pr-work-in-progress PR is still work in progress label Mar 4, 2019

eric-haibin-lin suggested changes Mar 6, 2019

View reviewed changes

ChaiBapchya added 2 commits April 9, 2019 01:59

Revert "fix commands to make doc consistent"

f54b321

This reverts commit 48f926f.

updated python doc

3d33bc6

ChaiBapchya requested a review from szha as a code owner April 10, 2019 09:15

aaronmarkham requested a review from eric-haibin-lin April 10, 2019 16:41

aaronmarkham reviewed Apr 10, 2019

View reviewed changes

line-too-long fix

4dced8c

eric-haibin-lin reviewed Apr 13, 2019

View reviewed changes

python/mxnet/ndarray_doc.py Show resolved Hide resolved

python/mxnet/ndarray_doc.py Show resolved Hide resolved

ChaiBapchya mentioned this pull request Apr 13, 2019

CI fails for unix cpu Static build CPU 14.04 #14174

Open

ChaiBapchya added 2 commits April 13, 2019 02:19

Merge branch 'master' into fix_ndarray_api_doc

9bbc18e

add empty line to render

bbcba2b

eric-haibin-lin previously requested changes Apr 17, 2019

View reviewed changes

python/mxnet/ndarray_doc.py Show resolved Hide resolved

rendering issue fix

d231b88

aaronmarkham requested review from aaronmarkham and eric-haibin-lin April 17, 2019 16:33

Trigger notification

8b50f96

aaronmarkham suggested changes Apr 26, 2019

View reviewed changes

Concat deprecated hence concatDoc

975ed0b

ChaiBapchya mentioned this pull request May 1, 2019

flaky test: test_ndarray_elementwisesum #14852

Open

ChaiBapchya added 2 commits May 2, 2019 01:51

Trigger notification

1dde088

Trigger notification

6bab23a

sandeep-krishnamurthy mentioned this pull request May 9, 2019

Add API documentation for upsampling operator with examples #14919

Merged

3 tasks

aaronmarkham approved these changes May 13, 2019

View reviewed changes

Trigger notification

ab412a2

aaronmarkham mentioned this pull request Jul 16, 2019

[Doc] broadcast axis is alias to broadcast axes #15546

Merged

4 tasks

Merge branch 'master' into fix_ndarray_api_doc

77c35bb

ChaiBapchya mentioned this pull request Jul 18, 2019

Flaky test test_metric.test_global_metric #15592

Closed

ChaiBapchya added 2 commits July 18, 2019 11:28

Trigger notification

83c5488

Trigger notification

e208c25

ChaiBapchya closed this Nov 5, 2019

ChaiBapchya deleted the fix_ndarray_api_doc branch November 5, 2019 01:37

ChaiBapchya mentioned this pull request Nov 5, 2019

Remove unused files in Website doc #16722

Merged

4 tasks


		Return the elements, either from x or y, depending on the condition.

		>>> x = mx.nd.array([[1, 2], [3, 4]])

Fix commands to make doc consistent #14243

Fix commands to make doc consistent #14243

Conversation

ChaiBapchya commented Feb 23, 2019 • edited Loading

aaronmarkham left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaronmarkham commented Feb 26, 2019

ChaiBapchya commented Feb 26, 2019 • edited Loading

aaronmarkham commented Feb 26, 2019

vandanavk commented Mar 4, 2019

aaronmarkham commented Mar 6, 2019

Choose a reason for hiding this comment

aaronmarkham commented Mar 6, 2019

eric-haibin-lin commented Mar 12, 2019

aaronmarkham commented Mar 12, 2019

eric-haibin-lin commented Mar 24, 2019

piyushghai commented Apr 9, 2019

ChaiBapchya commented Apr 9, 2019

eric-haibin-lin commented Apr 9, 2019

ChaiBapchya commented Apr 10, 2019

aaronmarkham left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChaiBapchya commented Apr 10, 2019

eric-haibin-lin left a comment

Choose a reason for hiding this comment

aaronmarkham commented Apr 19, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaronmarkham commented Apr 26, 2019

aaronmarkham left a comment

Choose a reason for hiding this comment

karan6181 commented May 21, 2019

ChaiBapchya commented Nov 5, 2019

ChaiBapchya commented Feb 23, 2019 •

edited

Loading

ChaiBapchya commented Feb 26, 2019 •

edited

Loading