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

Improve diagnose.py, adding build features info and binary library path. #15499

Merged
merged 1 commit into from
Jul 14, 2019

Conversation

larroy
Copy link
Contributor

@larroy larroy commented Jul 10, 2019

See title.

This is the script used to fill issues, so adding more info will help.

Refined output:

diagnose.txt

@larroy larroy requested a review from szha as a code owner July 10, 2019 00:01
@wkcn
Copy link
Member

wkcn commented Jul 12, 2019

Thank you for the improvement! LGTM.

Copy link
Member

@wkcn wkcn left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you : )

Copy link
Contributor

@apeforest apeforest left a comment

Choose a reason for hiding this comment

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

Would it cause any issue if user use this script to diagnose old mxnet release before 1.5?

Copy link
Contributor

@apeforest apeforest left a comment

Choose a reason for hiding this comment

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

Sorry, missed the try exception block in your code. It looks fine to me now.

@apeforest
Copy link
Contributor

One minor comment: the ✖ etc. symbols were not recognized when I import the txt file into MS office or Google Doc. Can we replace these unicode symbols by simple ascii, like Y/N?

Copy link
Contributor

@apeforest apeforest left a comment

Choose a reason for hiding this comment

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

Please make the output format compatible with MS office.

@apeforest apeforest self-requested a review July 13, 2019 05:48
@apeforest apeforest dismissed their stale review July 13, 2019 05:49

not blocking

@larroy
Copy link
Contributor Author

larroy commented Jul 14, 2019

I think they are valid unicode symbols, what problem are you experiencing with MS Office? I think that is out of scope of this PR as it requires changes to the Python code that prints the feature, feel free to send a different PR for that.

@larroy
Copy link
Contributor Author

larroy commented Jul 14, 2019

@apeforest I don't have any problems with Google docs or Office with those unicode characters.
Screen Shot 2019-07-13 at 7 56 35 PM
Screen Shot 2019-07-13 at 7 57 49 PM

@szha szha merged commit b25ec8e into apache:master Jul 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants