Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[tvmc] linting error on onnx command line driver frontend #6536

Merged
merged 1 commit into from Sep 24, 2020
Merged

[tvmc] linting error on onnx command line driver frontend #6536

merged 1 commit into from Sep 24, 2020

Conversation

leandron
Copy link
Contributor

@leandron leandron commented Sep 23, 2020 •
edited

Two minor changes on tvmc:

  • Updates onnx load function from "load" (a compatibility attribute) to "load_model" (the actual function)
  • Add a pylint comment, that we don't see currently on upstream CI, but it reproduces when linting it locally.

cc @manupa-arm @comaniac @d-smirnov

@leandron leandron changed the title tvmc: solve a linting error on onnx command line driver frontend tvmc: linting error on onnx command line driver frontend Sep 23, 2020
@leandron
Copy link
Contributor Author

also cc @tqchen to merge, in case you agree with this change.

@comaniac
Copy link
Contributor

From the ONNX package load is for backward compatibility. Is there any particular reason to change to load_model?
Also, pylint E1101 is no-member. I assume the reason you want to disable it in along with the change is because of the out-of-date lint file. Does that also imply it would be safer to use the compatible API?

@comaniac comaniac changed the title tvmc: linting error on onnx command line driver frontend [tvmc] linting error on onnx command line driver frontend Sep 23, 2020
@leandron
Copy link
Contributor Author

leandron commented Sep 23, 2020 •
edited

From the ONNX package load is for backward compatibility. Is there any particular reason to change to load_model?

Yeah, looking at it again, it makes sense to come back to load. Will revert it.

Also, pylint E1101 is no-member. I assume the reason you want to disable it in along with the change is because of the out-of-date lint file

This is the main reason for this PR. If I get a fresh clone from github and run ./docker/bash.sh tlcpack/ci-lint:v0.62 ./tests/scripts/task_lint.sh, I can see a pylint E1101, which seems correct.

Does that also imply it would be safer to use the compatible API?

Will move it back.

@leandron
tvmc: solve a linting error on onnx command line driver frontend
de455e6 
 * Updates onnx load function from "load" (a compatibility attribute)
   to "load_model" (the actual function)

 * Add a pylint command, that we don't see currently on upstream CI,
   but it reproduces when linting it locally.
@FrozenGene FrozenGene merged commit 22b8121 into apache:master Sep 24, 2020
@leandron leandron deleted the tvmc_lint branch September 24, 2020 06:50
@ZihengJiang ZihengJiang mentioned this pull request Sep 25, 2020
Closed
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Oct 13, 2020
@leandron
tvmc: solve a linting error on onnx command line driver frontend (apa…
3fc345a 
…che#6536)

* Updates onnx load function from "load" (a compatibility attribute)
   to "load_model" (the actual function)

 * Add a pylint command, that we don't see currently on upstream CI,
   but it reproduces when linting it locally.
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Oct 14, 2020
@leandron
tvmc: solve a linting error on onnx command line driver frontend (apa…
1a8f419 
…che#6536)

* Updates onnx load function from "load" (a compatibility attribute)
   to "load_model" (the actual function)

 * Add a pylint command, that we don't see currently on upstream CI,
   but it reproduces when linting it locally.
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Oct 15, 2020
@leandron
tvmc: solve a linting error on onnx command line driver frontend (apa…
b62965d 
…che#6536)

* Updates onnx load function from "load" (a compatibility attribute)
   to "load_model" (the actual function)

 * Add a pylint command, that we don't see currently on upstream CI,
   but it reproduces when linting it locally.
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Oct 15, 2020
@leandron
tvmc: solve a linting error on onnx command line driver frontend (apa…
edf018b 
…che#6536)

* Updates onnx load function from "load" (a compatibility attribute)
   to "load_model" (the actual function)

 * Add a pylint command, that we don't see currently on upstream CI,
   but it reproduces when linting it locally.
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Oct 16, 2020
@leandron
tvmc: solve a linting error on onnx command line driver frontend (apa…
e91f8d2 
…che#6536)

* Updates onnx load function from "load" (a compatibility attribute)
   to "load_model" (the actual function)

 * Add a pylint command, that we don't see currently on upstream CI,
   but it reproduces when linting it locally.
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Oct 19, 2020
@leandron @trevor-m
tvmc: solve a linting error on onnx command line driver frontend (apa…
cb1d58b 
…che#6536)

* Updates onnx load function from "load" (a compatibility attribute)
   to "load_model" (the actual function)

 * Add a pylint command, that we don't see currently on upstream CI,
   but it reproduces when linting it locally.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tqchen tqchen tqchen approved these changes

@comaniac comaniac comaniac approved these changes

@mbaret mbaret mbaret approved these changes

@manupak manupak manupak approved these changes

Assignees

@comaniac comaniac

Labels
status: accepted
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@leandron @comaniac @tqchen @mbaret @manupak @FrozenGene