areusch commented on code in PR #12028:
URL: https://github.com/apache/tvm/pull/12028#discussion_r929114832
##########
tests/python/frontend/oneflow/test_vision_models.py:
##########
@@ -14,7 +14,7 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-# pylint: disable=import-self, invalid-name
+# pylint: disable=import-self, invalid-name, consider-using-f-string
Review Comment:
could you say why this one is disabled?
##########
tests/python/frontend/tensorflow/test_forward.py:
##########
@@ -14,29 +14,27 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-# pylint: disable=import-self, invalid-name, unused-argument
+# pylint: disable=import-self, invalid-name, unused-argument, unused-variable,
redefined-builtin
Review Comment:
can you say why you're disabling these extras?
##########
tests/python/frontend/tflite/test_forward.py:
##########
@@ -2557,14 +2579,15 @@ def _test_forward_add_n(inputs):
temp.append(tf.placeholder(shape=each.shape, dtype=each.dtype))
output = tf.add_n(temp)
compare_tflite_with_tvm(
- [each for each in inputs],
+ [each for each in inputs], # pylint:
disable=unnecessary-comprehension
Review Comment:
list(inputs), here and below
##########
tests/python/frontend/keras/test_forward.py:
##########
@@ -125,7 +125,7 @@ def get_mobilenet(keras):
@tvm.testing.uses_gpu
-class TestKeras:
+class TestKeras: # pylint: disable=missing-class-docstring
Review Comment:
can you just add a brief docstring instead?
##########
tests/python/frontend/tensorflow/test_forward.py:
##########
@@ -498,7 +501,7 @@ def _test_convolution(
add_shapes_to_graph_def=True,
):
"""One iteration of convolution with given shapes and attributes"""
-
+ # pylint: disable=dangerous-default-value
Review Comment:
please don't disable this one, it can lead to very insidious false test
results. here and below.
##########
tests/python/frontend/tflite/test_forward.py:
##########
@@ -1831,6 +1851,7 @@ def test_forward_concatenation():
def _test_unary_elemwise(math_op, data, quantized, quant_range=[-6, 6],
int_quant_dtype=tf.int8):
"""One iteration of unary elemwise"""
+ # pylint: disable= dangerous-default-value
Review Comment:
don't disable this one--instead say `quant_range=(-6, 6)`
##########
tests/python/frontend/pytorch/test_forward.py:
##########
@@ -227,13 +230,14 @@ def verify_model_with_input(
compiled_output = relay_model.get_output(0).numpy()
assert_shapes_match(baseline_outputs, compiled_output)
- if assert_shape_only == False:
+ if assert_shape_only is False:
tvm.testing.assert_allclose(baseline_outputs, compiled_output,
rtol=rtol, atol=atol)
# Single operator tests
@tvm.testing.uses_gpu
def test_forward_pixel_shuffle():
+ """PixelShuffle"""
Review Comment:
per the previous discussion, can you adjust the docstrings to match the
function name?
##########
tests/python/frontend/tflite/test_forward.py:
##########
@@ -14,21 +14,30 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-# pylint: disable=import-self, invalid-name, unused-argument
+# pylint: disable=import-self, invalid-name, unused-argument, unused-variable
Review Comment:
similar question with these
##########
tests/python/frontend/tflite/test_forward.py:
##########
@@ -1777,16 +1791,22 @@ def test_forward_shape():
limit = tf.placeholder(dtype=tf.int32, shape=[], name="limit")
delta = tf.placeholder(dtype=tf.int32, shape=[], name="delta")
r = tf.range(start, limit, delta, tf.int32, name="range")
- out = tf.shape(r, out_type=tf.dtypes.int32)
+ out = tf.shape(r, out_type=dtype)
+ out = tf.add(out, tf.constant([1], dtype=dtype))
compare_tflite_with_tvm(
- [x for x in np.nditer(data)],
+ [x for x in np.nditer(data)], # pylint:
disable=unnecessary-comprehension
Review Comment:
can we do list(np.nditer(data)) to avoid disabling?
##########
tests/python/frontend/darknet/test_forward.py:
##########
@@ -23,16 +24,16 @@
"""
import numpy as np
import tvm
-from tvm import te
from tvm.contrib import graph_executor
from tvm.contrib.download import download_testdata
-download_testdata.__test__ = False
from tvm.relay.testing.darknet import LAYERTYPE
from tvm.relay.testing.darknet import __darknetffi__
from tvm.relay.frontend.darknet import ACTIVATION
from tvm import relay
+download_testdata.__test__ = False
Review Comment:
yeah i believe so, i tried it locally and it seems to be ok for this
particular test. it seems like that's designed to tell pytest that
download_testdata isn't a test, but i'm not sure why it would think that to
begin with.
##########
tests/python/frontend/caffe/test_forward.py:
##########
@@ -37,9 +32,13 @@
import tvm
import tvm.testing
from tvm import relay
-from tvm.contrib import utils, graph_executor
+from tvm.contrib import graph_executor
from tvm.contrib.download import download_testdata
+os.environ["GLOG_minloglevel"] = "2"
Review Comment:
ah sorry, my mistake; nevermind
##########
tests/python/frontend/tensorflow/test_forward.py:
##########
@@ -294,7 +297,7 @@ def is_gpu_available():
local_device_protos = device_lib.list_local_devices()
gpu_list = [x.name for x in local_device_protos if x.device_type == "GPU"]
- if len(gpu_list) > 0:
+ if len(gpu_list) > 0: # pylint: disable=len-as-condition
Review Comment:
given the list is initialized on previous like, i think you can adopt the
preferred syntax `if gpu_list:` here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
