Skip to content

Conversation

zhenhuaw-me
Copy link
Member

@zhenhuaw-me zhenhuaw-me commented May 4, 2021

We need to setup env var ONNX_ML and run gen_doc.py twice to update the operator docs if both ONNX and ONNX-ML doc need to be updated.

With this change, python onnx/defs/gen_doc.py generates ONNX and ONNX-ML doc in one run.

@zhenhuaw-me zhenhuaw-me requested review from a team as code owners May 4, 2021 13:51
Copy link
Collaborator

@askhade askhade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a few comments. Most are nit picks... 1 regarding type info needs to be fixed to turn CIs green

Zhenhua Wang (王振华) added 2 commits May 5, 2021 12:46
We need to setup env var `ONNX_ML` and run `gen_doc.py` twice
to update the operator docs if both _ONNX_ and _ONNX-ML_ doc
need to be updated.

With this change, `python onnx/defs/gen_doc.py` generates
_ONNX_ and _ONNX-ML_ doc in one run.

Signed-off-by: Zhenhua Wang (王振华) <zhenhua.wzh@icloud.com>
Signed-off-by: Zhenhua Wang (王振华) <zhenhua.wzh@icloud.com>
@zhenhuaw-me
Copy link
Member Author

Hi @askhade thank you for the quick review. I have fixed your concerns, please help to check.

@zhenhuaw-me zhenhuaw-me requested a review from askhade May 5, 2021 12:48
Signed-off-by: Zhenhua Wang (王振华) <zhenhua.wzh@icloud.com>
@zhenhuaw-me
Copy link
Member Author

Hmmm... Cannot reproduce the error with python 3.8. When trying to repro with the steps of the pipeline onnx-lite build, fail to build. Is it necessary to build protobuf online for the CI test?

@zhenhuaw-me
Copy link
Member Author

Close since run twice for ONNX and ONNX-ML should be by design. Thanks for the review!

@askhade
Copy link
Collaborator

askhade commented May 18, 2021

@jackwish : I think you missunderstood the comment. Your PR is a good enhancement for the current scenario it just needs a little change.

When ONNX_ML is set to 0 when building onnx we do not build ml ops and therefore if you run the gen doc script it will create an empty operators-ml.md file. This is what needs to be fixed. In CIs we do set ONNX_ML to 0 and after running gen_doc.py it produces as empty operators-ml.md and therefore CIs are failing.

I hope you will reopen it and make this fix.

xadupre added a commit that referenced this pull request Jul 4, 2023
### Description
<!-- - Describe your changes. -->
Generate both onnx and onnx-ml operator docs when ONNX_ML=1

### Motivation and Context
<!-- - Why is this change required? What problem does it solve? -->
<!-- - If it fixes an open issue, please link to the issue here. -->
Closes #5377. Similar to
#3467.

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Co-authored-by: Xavier Dupré <xadupre@users.noreply.github.com>
hamptonm1 pushed a commit to hamptonm1/onnx that referenced this pull request Jul 10, 2023
### Description
<!-- - Describe your changes. -->
Generate both onnx and onnx-ml operator docs when ONNX_ML=1

### Motivation and Context
<!-- - Why is this change required? What problem does it solve? -->
<!-- - If it fixes an open issue, please link to the issue here. -->
Closes onnx#5377. Similar to
onnx#3467.

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Co-authored-by: Xavier Dupré <xadupre@users.noreply.github.com>
Signed-off-by: Megan Hampton <hamptonm@us.ibm.com>
adityagoel4512 pushed a commit to adityagoel4512/onnx that referenced this pull request Jul 28, 2023
### Description
<!-- - Describe your changes. -->
Generate both onnx and onnx-ml operator docs when ONNX_ML=1

### Motivation and Context
<!-- - Why is this change required? What problem does it solve? -->
<!-- - If it fixes an open issue, please link to the issue here. -->
Closes onnx#5377. Similar to
onnx#3467.

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Co-authored-by: Xavier Dupré <xadupre@users.noreply.github.com>
Signed-off-by: Aditya Goel <agoel4512@gmail.com>
adityagoel4512 pushed a commit to adityagoel4512/onnx that referenced this pull request Jul 28, 2023
### Description
<!-- - Describe your changes. -->
Generate both onnx and onnx-ml operator docs when ONNX_ML=1

### Motivation and Context
<!-- - Why is this change required? What problem does it solve? -->
<!-- - If it fixes an open issue, please link to the issue here. -->
Closes onnx#5377. Similar to
onnx#3467.

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Co-authored-by: Xavier Dupré <xadupre@users.noreply.github.com>
Signed-off-by: Aditya Goel <agoel4512@gmail.com>
adityagoel4512 pushed a commit to adityagoel4512/onnx that referenced this pull request Jul 28, 2023
### Description
<!-- - Describe your changes. -->
Generate both onnx and onnx-ml operator docs when ONNX_ML=1

### Motivation and Context
<!-- - Why is this change required? What problem does it solve? -->
<!-- - If it fixes an open issue, please link to the issue here. -->
Closes onnx#5377. Similar to
onnx#3467.

Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
Co-authored-by: Xavier Dupré <xadupre@users.noreply.github.com>
Signed-off-by: Aditya Goel <agoel4512@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants