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

[MXNET-1291] solve pylint errors in examples with issue no.12205 #13938

Merged
merged 10 commits into from
Mar 14, 2019

Conversation

cchung100m
Copy link
Contributor

@cchung100m cchung100m commented Jan 20, 2019

Description

this is the part 3 of the changes to solve pylint errors in examples with issue #12205

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
  • Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
  • Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
  • Code is well-documented:
  • For user-facing API changes, API doc string has been updated.
  • For new C++ functions in header files, their functionalities and arguments are documented.
  • For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
  • Check the API doc at http:https://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • Feature1, tests, (and when applicable, API doc)
  • Feature2, tests, (and when applicable, API doc)

Comments

  • If this change is a backward incompatible change, why must this change be made.
  • Interesting edge cases to note here

@cchung100m cchung100m requested a review from szha as a code owner January 20, 2019 04:32
example/bayesian-methods/bdk_demo.py Outdated Show resolved Hide resolved
example/bayesian-methods/bdk_demo.py Outdated Show resolved Hide resolved
example/caffe/caffe_net.py Outdated Show resolved Hide resolved
example/caffe/train_model.py Outdated Show resolved Hide resolved
example/caffe/train_model.py Outdated Show resolved Hide resolved
example/cnn_text_classification/data_helpers.py Outdated Show resolved Hide resolved
example/cnn_text_classification/data_helpers.py Outdated Show resolved Hide resolved
example/ctc/multiproc_data.py Show resolved Hide resolved
example/gluon/dc_gan/dcgan.py Outdated Show resolved Hide resolved
example/gluon/lstm_crf/lstm_crf.py Outdated Show resolved Hide resolved
Copy link
Contributor

@ChaiBapchya ChaiBapchya left a comment

Choose a reason for hiding this comment

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

Branch has a merge conflict. Please resolve. Thanks a lot for this really big contribution! Great work!

@cchung100m
Copy link
Contributor Author

@ChaiBapchya ,
I solve the conflict of bdk_demo.py and re-trigger the CI. thank you for the suggestions. :)

@marcoabreu
Copy link
Contributor

Thanks a lot for your contribution. Could you find out why our sanity check did not catch this? Also, it seems like you are deleting a bunch of files

@cchung100m
Copy link
Contributor Author

Hi @marcoabreu

Sorry, I don’t know what you want to tell me. Can you elaborate on that?

Many thanks,

example/autoencoder/model.py Outdated Show resolved Hide resolved
example/autoencoder/solver.py Outdated Show resolved Hide resolved
example/bayesian-methods/algos.py Outdated Show resolved Hide resolved
@@ -1,100 +0,0 @@
# Urban Sounds Classification in MXNet Gluon
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I don't know anything about this file, I just accept this change which made by communities before committing my branch.

@sandeep-krishnamurthy
Copy link
Contributor

@cchung100m - Thank you for your contributions and solving pylint issues in examples. I have left few comments.
How are we testing that examples are not broken after this change? @marcoabreu

@marcoabreu
Copy link
Contributor

We don't test examples in CI :(

Copy link
Contributor

@marcoabreu marcoabreu left a comment

Choose a reason for hiding this comment

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

Please update out pylint pipeline to ensure all these points are caught in future

@vandanavk
Copy link
Contributor

@cchung100m are the files in example/autoencoder being deleted?

@marcoabreu we might have to wait for the fixes to all the lint errors (in examples folder) to be merged, before we enable the check in CI

@ankkhedia
Copy link
Contributor

@cchung100m Could you please look into the comments above?

@cchung100m
Copy link
Contributor Author

@vandanavk
Sorry for the late reply.
Yes, the files in example/autoencoder being deleted.

@ankkhedia
Thanks for the reminder.

I will solve the conflict files and rebase this branch ASAP.

Copy link
Contributor

@vandanavk vandanavk left a comment

Choose a reason for hiding this comment

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

LGTM

@vandanavk
Copy link
Contributor

@marcoabreu @ChaiBapchya @sandeep-krishnamurthy is this PR good to go?

@cchung100m
Copy link
Contributor Author

I will re-check the conflicting files ASAP.

Unify the style of comments
Unify the style of comments
Correct the misuse of tenses from preprocessed to preprocesses
1. Add reference of the paper in comment.
2. Change the comments from  "Run training to CapsNet" to "Perform CapsNet training"
Remove a redundant noun 'dataset'
Align the style of comment from single-line comment to multi-line comment
Correct the misdescription comment of LogSoftmax class from 'softmax loss function' to 'logarithm of softmax'
Remove the '#' symbol in the header comment
update out pylint pipeline to ensure all these points are caught
@cchung100m
Copy link
Contributor Author

Hi @szha @marcoabreu @ChaiBapchya @sandeep-krishnamurthy

I complete the rebase and re-test on this branch, I will appreciate if you can help to review/merge, again, thanks :)

@marcoabreu marcoabreu merged commit 9fd3153 into apache:master Mar 14, 2019
vdantu pushed a commit to vdantu/incubator-mxnet that referenced this pull request Mar 31, 2019
…che#13938)

* Unify the style of comments

Unify the style of comments

* Unify the style of comments

Unify the style of comments

* Correct the misuse of tenses 

Correct the misuse of tenses from preprocessed to preprocesses

* Enhance the comment for better description

1. Add reference of the paper in comment.
2. Change the comments from  "Run training to CapsNet" to "Perform CapsNet training"

* Remove a redundant noun

Remove a redundant noun 'dataset'

* Unify the style of comments

Align the style of comment from single-line comment to multi-line comment

* Correct the misdescription of LogSoftmax class

Correct the misdescription comment of LogSoftmax class from 'softmax loss function' to 'logarithm of softmax'

* Remove the '#' symbol in the header comment

Remove the '#' symbol in the header comment

* update the result of pylint pipeline

update out pylint pipeline to ensure all these points are caught

* retrigger the CI
nswamy pushed a commit that referenced this pull request Apr 5, 2019
)

* Unify the style of comments

Unify the style of comments

* Unify the style of comments

Unify the style of comments

* Correct the misuse of tenses 

Correct the misuse of tenses from preprocessed to preprocesses

* Enhance the comment for better description

1. Add reference of the paper in comment.
2. Change the comments from  "Run training to CapsNet" to "Perform CapsNet training"

* Remove a redundant noun

Remove a redundant noun 'dataset'

* Unify the style of comments

Align the style of comment from single-line comment to multi-line comment

* Correct the misdescription of LogSoftmax class

Correct the misdescription comment of LogSoftmax class from 'softmax loss function' to 'logarithm of softmax'

* Remove the '#' symbol in the header comment

Remove the '#' symbol in the header comment

* update the result of pylint pipeline

update out pylint pipeline to ensure all these points are caught

* retrigger the CI
haohuanw pushed a commit to haohuanw/incubator-mxnet that referenced this pull request Jun 23, 2019
…che#13938)

* Unify the style of comments

Unify the style of comments

* Unify the style of comments

Unify the style of comments

* Correct the misuse of tenses 

Correct the misuse of tenses from preprocessed to preprocesses

* Enhance the comment for better description

1. Add reference of the paper in comment.
2. Change the comments from  "Run training to CapsNet" to "Perform CapsNet training"

* Remove a redundant noun

Remove a redundant noun 'dataset'

* Unify the style of comments

Align the style of comment from single-line comment to multi-line comment

* Correct the misdescription of LogSoftmax class

Correct the misdescription comment of LogSoftmax class from 'softmax loss function' to 'logarithm of softmax'

* Remove the '#' symbol in the header comment

Remove the '#' symbol in the header comment

* update the result of pylint pipeline

update out pylint pipeline to ensure all these points are caught

* retrigger the CI
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI pr-awaiting-review PR is waiting for code review Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants