Fix Python formatting #3988

stiepan · 2022-06-15T09:22:56Z

Category:

Other (e.g. Documentation, Tests, Configuration)

Description:

Additional information:

Affected modules and functionalities:

Key points relevant for the review:

Tests:

Checklist

Documentation

DALI team only

Requirements

Implements new requirements
Affects existing requirements
N/A

REQ IDs: N/A

JIRA TASK: DALI-2823

stiepan · 2022-06-15T09:57:28Z

!build

dali-automaton · 2022-06-15T10:00:05Z

CI MESSAGE: [5089788]: BUILD STARTED

mzient · 2022-06-15T10:51:34Z

dali/test/python/test_dali_tf_dataset_shape.py

@@ -166,7 +172,9 @@ def define_graph(self):

 def test_multiple_input_valid_shapes():
    for batch in [1, 10]:
-        for shapes in [None, (None, None), ((batch, 200, 200, 3), None), (None, (batch, 1)), (None, (batch,))]:
+        for shapes in [
+            None, (None, None), ((batch, 200, 200, 3), None), (None, (batch, 1)), (None, (batch,))


Actually, given the number of commas and braces:

Suggested change

None, (None, None), ((batch, 200, 200, 3), None), (None, (batch, 1)), (None, (batch,))

None,

(None, None),

((batch, 200, 200, 3), None),

(None, (batch, 1)),

(None, (batch,))

mzient · 2022-06-15T10:52:13Z

dali/test/python/test_dali_tf_dataset_shape.py

-        for shape in [(None, None, None, None), (None, None, 2), (batch, None, None, None),
-                    (batch, None, 2)]:
+        for shape in [
+            (None, None, None, None), (None, None, 2), (batch, None, None, None), (batch, None, 2)


Suggested change

(None, None, None, None), (None, None, 2), (batch, None, None, None), (batch, None, 2)

(None, None, None, None),

(None, None, 2),

(batch, None, None, None),

(batch, None, 2)

mzient · 2022-06-15T10:53:28Z

dali/test/python/test_dali_tf_dataset_shape.py

+    yield raises(TypeError, expected_msg)(dali_pipe_deprecated), {"shapes": 2}, 2, tf.uint8, \
+        dali_types.UINT8, 1, 2


Suggested change

yield raises(TypeError, expected_msg)(dali_pipe_deprecated), {"shapes": 2}, 2, tf.uint8, \

dali_types.UINT8, 1, 2

yield raises(TypeError, expected_msg)(dali_pipe_deprecated), \

{"shapes": 2}, 2, tf.uint8, dali_types.UINT8, 1, 2

mzient · 2022-06-15T10:54:25Z

dali/test/python/test_dali_variable_batch_size.py

+    data = generate_data(31, 13, custom_shape_generator(3, 7, 160, 200, 80, 100, 3, 3), lo=0,
+                         hi=255, dtype=np.uint8)


Suggested change

data = generate_data(31, 13, custom_shape_generator(3, 7, 160, 200, 80, 100, 3, 3), lo=0,

hi=255, dtype=np.uint8)

data = generate_data(31, 13, custom_shape_generator(3, 7, 160, 200, 80, 100, 3, 3),

lo=0, hi=255, dtype=np.uint8)

dali-automaton · 2022-06-15T11:00:49Z

CI MESSAGE: [5089788]: BUILD FAILED

mzient · 2022-06-15T11:14:16Z

dali/test/python/test_dali_variable_batch_size.py

+        'batch_processing': True, 'devices': ['cpu'], 'in_types': [types.UINT8],
+        'ins_ndim': [3], 'out_types': [types.UINT8],  'outs_ndim': [3],


Suggested change

'batch_processing': True, 'devices': ['cpu'], 'in_types': [types.UINT8],

'ins_ndim': [3], 'out_types': [types.UINT8], 'outs_ndim': [3],

'batch_processing': True, 'devices': ['cpu'],

'in_types': [types.UINT8], 'ins_ndim': [3],

'out_types': [types.UINT8], 'outs_ndim': [3],

mzient · 2022-06-15T11:14:32Z

dali/test/python/test_dali_variable_batch_size.py

+        'batch_processing': False, 'devices': ['cpu'], 'in_types': [types.UINT8],
+        'ins_ndim': [3], 'out_types': [types.UINT8],  'outs_ndim': [3],


Suggested change

'batch_processing': False, 'devices': ['cpu'], 'in_types': [types.UINT8],

'ins_ndim': [3], 'out_types': [types.UINT8], 'outs_ndim': [3],

'batch_processing': False, 'devices': ['cpu'],

'in_types': [types.UINT8], 'ins_ndim': [3],

'out_types': [types.UINT8], 'outs_ndim': [3],

mzient · 2022-06-15T11:46:16Z

dali/test/python/test_operator_erase.py

+    rois = [
+        ("HWC", (60, 80, 3), "HW", (4, 10), (40, 50), (4 / 60.0, 10 / 80.0), (40 / 60.0, 50 / 80.0),
+         0),
+        ("HWC", (60, 80, 3), "HW", (4, 10), (40, 50), (4 / 60.0, 10 / 80.0), (40 / 60.0, 50 / 80.0),
+         -1),
+        ("HWC", (60, 80, 3), "HW", (4, 10), (40, 50), (4 / 60.0, 10 / 80.0), (40 / 60.0, 50 / 80.0),
+         (118, 186, 0)),
+        ("HWC", (60, 80, 3), "HW", (4, 2, 3, 4), (50, 10, 10, 50),
+         (4 / 60.0, 2 / 80.0, 3 / 60.0, 4 / 80.0), (50 / 60.0, 10 / 80.0, 10 / 60.0, 50 / 80.0), 0),
+        ("HWC", (60, 80, 3), "H", (4, ), (7, ), (4 / 60.0, ), (7 / 60.0, ), 0),
+        ("DHWC", (10, 60, 80, 3), "DHW", (2, 4, 10), (5, 40, 50), (2 / 10.0, 4 / 60.0, 10 / 80.0),
+         (5 / 10.0, 40 / 60.0, 50 / 80.0), 0),
+        ("HWC", (60, 80, 1), "WH", (10, 4), (50, 40), (10 / 80.0, 4 / 60.0), (50 / 80.0, 40 / 60.0),
+         0),
+        ("XYZ", (60, 80, 3), "X", (4, ), (7, ), (4 / 60.0, ), (7 / 60.0, ), -10),


This is terrible - I'd recommend reverting and adding #noqa or formatting by hand, e.g:

Suggested change

rois = [

("HWC", (60, 80, 3), "HW", (4, 10), (40, 50), (4 / 60.0, 10 / 80.0), (40 / 60.0, 50 / 80.0),

0),

("HWC", (60, 80, 3), "HW", (4, 10), (40, 50), (4 / 60.0, 10 / 80.0), (40 / 60.0, 50 / 80.0),

-1),

("HWC", (60, 80, 3), "HW", (4, 10), (40, 50), (4 / 60.0, 10 / 80.0), (40 / 60.0, 50 / 80.0),

(118, 186, 0)),

("HWC", (60, 80, 3), "HW", (4, 2, 3, 4), (50, 10, 10, 50),

(4 / 60.0, 2 / 80.0, 3 / 60.0, 4 / 80.0), (50 / 60.0, 10 / 80.0, 10 / 60.0, 50 / 80.0), 0),

("HWC", (60, 80, 3), "H", (4, ), (7, ), (4 / 60.0, ), (7 / 60.0, ), 0),

("DHWC", (10, 60, 80, 3), "DHW", (2, 4, 10), (5, 40, 50), (2 / 10.0, 4 / 60.0, 10 / 80.0),

(5 / 10.0, 40 / 60.0, 50 / 80.0), 0),

("HWC", (60, 80, 1), "WH", (10, 4), (50, 40), (10 / 80.0, 4 / 60.0), (50 / 80.0, 40 / 60.0),

0),

("XYZ", (60, 80, 3), "X", (4, ), (7, ), (4 / 60.0, ), (7 / 60.0, ), -10),

rois = [

("HWC", (60, 80, 3), "HW", (4, 10), (40, 50),

(4 / 60.0, 10 / 80.0), (40 / 60.0, 50 / 80.0), 0),

("HWC", (60, 80, 3), "HW", (4, 10), (40, 50),

(4 / 60.0, 10 / 80.0), (40 / 60.0, 50 / 80.0), -1),

("HWC", (60, 80, 3), "HW", (4, 10), (40, 50),

(4 / 60.0, 10 / 80.0), (40 / 60.0, 50 / 80.0), (118, 186, 0)),

("HWC", (60, 80, 3), "HW", (4, 2, 3, 4), (50, 10, 10, 50),

(4 / 60.0, 2 / 80.0, 3 / 60.0, 4 / 80.0), (50 / 60.0, 10 / 80.0, 10 / 60.0, 50 / 80.0), 0),

("HWC", (60, 80, 3), "H", (4, ), (7, ), (4 / 60.0, ), (7 / 60.0, ), 0),

("DHWC", (10, 60, 80, 3), "DHW", (2, 4, 10), (5, 40, 50),

(2 / 10.0, 4 / 60.0, 10 / 80.0), (5 / 10.0, 40 / 60.0, 50 / 80.0), 0),

("HWC", (60, 80, 1), "WH", (10, 4), (50, 40),

(10 / 80.0, 4 / 60.0), (50 / 80.0, 40 / 60.0), 0),

("XYZ", (60, 80, 3), "X", (4, ), (7, ), (4 / 60.0, ), (7 / 60.0, ), -10),

mzient · 2022-06-15T11:46:44Z

dali/test/python/test_operator_erase.py

+            for (input_layout, input_shape, axis_names, anchor, shape, anchor_norm,
+                 shape_norm, fill_value) in rois:


Suggested change

for (input_layout, input_shape, axis_names, anchor, shape, anchor_norm,

shape_norm, fill_value) in rois:

for (input_layout, input_shape, axis_names, anchor, shape,

anchor_norm, shape_norm, fill_value) in rois:

mzient · 2022-06-15T11:47:01Z

dali/test/python/test_operator_erase.py

+                        input_shape, anchor, shape, axis_names, input_layout, anchor_norm_arg, \
+                        shape_norm_arg, normalized_anchor, normalized_shape


Suggested change

input_shape, anchor, shape, axis_names, input_layout, anchor_norm_arg, \

shape_norm_arg, normalized_anchor, normalized_shape

input_shape, anchor, shape, axis_names, input_layout,

anchor_norm_arg, shape_norm_arg, normalized_anchor, normalized_shape

mzient · 2022-06-15T11:47:45Z

dali/test/python/test_operator_erase.py

+    yield check_operator_erase_with_normalized_coords, device, batch_size, input_shape, anchor_arg,\
+        shape_arg, axis_names, input_layout, anchor_norm_arg, shape_norm_arg, True, True


Suggested change

yield check_operator_erase_with_normalized_coords, device, batch_size, input_shape, anchor_arg,\

shape_arg, axis_names, input_layout, anchor_norm_arg, shape_norm_arg, True, True

yield (check_operator_erase_with_normalized_coords,

device,

batch_size,

input_shape,

anchor_arg,

shape_arg,

axis_names,

input_layout,

anchor_norm_arg,

shape_norm_arg,

True,

True)

or

Suggested change

yield check_operator_erase_with_normalized_coords, device, batch_size, input_shape, anchor_arg,\

shape_arg, axis_names, input_layout, anchor_norm_arg, shape_norm_arg, True, True

yield (check_operator_erase_with_normalized_coords,

device, batch_size,

input_shape,

anchor_arg, shape_arg,

axis_names, input_layout,

anchor_norm_arg, shape_norm_arg,

True, True)

mzient · 2022-06-15T11:49:50Z

dali/test/python/test_operator_join.py

+    ref = np.array([[1, 2, 3, 4, 13, 14, 15],
+                    [5, 6, 7, 8, 16, 17, 18],
+                    [9, 10, 11, 12, 19, 20, 21]])


Would that be ok? I wanted this to be aligned in columns.

Suggested change

ref = np.array([[1, 2, 3, 4, 13, 14, 15],

[5, 6, 7, 8, 16, 17, 18],

[9, 10, 11, 12, 19, 20, 21]])

ref = np.array([[1, 2, 3, 4, 13, 14, 15],

[5, 6, 7, 8, 16, 17, 18],

[9, 10, 11, 12, 19, 20, 21]])

mzient · 2022-06-15T11:50:37Z

dali/test/python/test_operator_join.py

+    ref = np.array([[1, 2, 3, 4, 13, 14, 15],
+                    [5, 6, 7, 8, 16, 17, 18],
+                    [9, 10, 11, 12, 19, 20, 21]])


Suggested change

ref = np.array([[1, 2, 3, 4, 13, 14, 15],

[5, 6, 7, 8, 16, 17, 18],

[9, 10, 11, 12, 19, 20, 21]])

ref = np.array([[1, 2, 3, 4, 13, 14, 15],

[5, 6, 7, 8, 16, 17, 18],

[9, 10, 11, 12, 19, 20, 21]])

jantonguirao · 2022-06-15T10:04:57Z

dali/test/python/test_backend_impl_gpu.py

@@ -82,108 +84,110 @@ def test_cuda_array_interface_tensor_gpu():
    pipe.build()
    tensor_list = pipe.run()[0]
    assert tensor_list[0].__cuda_array_interface__['data'][0] == tensor_list[0].data_ptr()
-    assert tensor_list[0].__cuda_array_interface__['data'][1] == True
+    assert tensor_list[0].__cuda_array_interface__['data'][1] is True


Suggested change

assert tensor_list[0].__cuda_array_interface__['data'][1] is True

assert tensor_list[0].__cuda_array_interface__['data'][1]

As far as I understand, the 'data' is expected to be a tuple of integer and boolean, so when choosing on how to fix I leant to a check that does not rely on implicit conversion to bool

jantonguirao · 2022-06-15T10:08:46Z

dali/test/python/test_dali_variable_batch_size.py

    exclude = re.compile(exclude)
    methods = [x for x in methods if not exclude.match(x)]
    # we are fine with covering more we can easily list, like numba
-    assert set(methods).difference(set(tested_methods)) == set(), "Test doesn't cover:\n {}".format(set(methods) - set(tested_methods))
+    assert set(methods).difference(set(tested_methods)) == set(), "Test doesn't cover:\n {}".format(


might be personal preference. I'd have broken the line before the string

jantonguirao · 2022-06-17T07:18:23Z

dali/test/python/test_operator_erase.py

+    anchor_norm_arg = (
+        4 / 60.0, 10 / 80.0, 2000, 2000, 10 / 60.0, 4 / 80.0
+    )  # second region is completely out of bounds


Personal preference: I'd put the comment in the previous line and keep the tuple definition in one line

jantonguirao · 2022-06-17T07:19:59Z

dali/test/python/test_operator_mfcc.py

+        for dct_type, norm, axis, n_mfcc, lifter, shape, msg in [
+            # DCT-I ortho-normalization is not supported
+            (1, True, 0, 20, 0.0, (100, 100),
+             "Ortho-normalization is not supported for DCT type I"),
+            # axis out of bounds
+            (2, False, -1, 20, 0.0, (100, 100), "Provided axis cannot be negative"),
+            # axis out of bounds
+            (2, False, 2, 20, 0.0, (100, 100), "Axis [\\d]+ is out of bounds \\[[\\d]+,[\\d]+\\)"),
+            # not supported DCT type
+            (10, False, 0, 20, 0.0, (100, 100),
+             "Unsupported DCT type: 10. Supported types are: 1, 2, 3, 4"),
+        ]:


I'd aim for some consistency here (e.g. move the error message to the next line always)

jantonguirao · 2022-06-17T07:22:37Z

dali/test/python/test_pipeline.py

-        assert(True)
-    except BaseException:
-        assert(False)
+    lazy_pipe.build()


missing assert_raises here?

The example above had different order of asserts, I think this one was meant to simply pass (as opposed to the one above that should raise).

jantonguirao · 2022-06-17T07:25:21Z

dali/test/python/test_pipeline.py

+        out1 = as_cpu(out[0]).at(i)
+        out2 = as_cpu(out[1]).at(i)
+        out3 = as_cpu(out[2]).at(i)


Consider test_utils.as_array(out[0][i]). You wouldn't need as_cpu

Cool, thanks!

jantonguirao · 2022-06-17T07:25:38Z

dali/test/python/test_pipeline.py

-        out1 = out[0].as_cpu().at(i) if isinstance(out[0][0], dali.backend_impl.TensorGPU) else out[0].at(i)
-        out2 = out[1].as_cpu().at(i) if isinstance(out[1][0], dali.backend_impl.TensorGPU) else out[1].at(i)
-        out3 = out[2].as_cpu().at(i) if isinstance(out[2][0], dali.backend_impl.TensorGPU) else out[2].at(i)
+        out1 = as_cpu(out[0]).at(i)


Consider test_utils.as_array

jantonguirao · 2022-06-17T07:26:22Z

dali/test/python/test_pipeline.py

@@ -1656,42 +1701,42 @@ def test_preserve_arg():
    pipe = dali.Pipeline(1, 1, 0)
    with pipe:
        out = dali.fn.external_source(source=[[np.array([-0.5, 1.25])]], preserve=True)
-        res = dali.fn.resize(out, preserve=True)
+        res = dali.fn.resize(out, preserve=True)  # noqa: F841


out of curiosity. What's the problem here?

Oh, maybe it is silly: the test checks preservation of unused operators if preserve is set to True. So the linter complains about unused variable. I could very well just call resize and not assign the result to any variable, but wanted to preserve this more "canonical" look of defining the pipeline, after all the variable is unused intentionally.

jantonguirao · 2022-06-17T07:26:56Z

dali/test/python/test_pipeline.py

+        assert pipe.exec_pipelined is True
+        assert pipe.exec_async is True
+        assert pipe.set_affinity is True


Suggested change

assert pipe.exec_pipelined is True

assert pipe.exec_async is True

assert pipe.set_affinity is True

assert pipe.exec_pipelined is True

assert pipe.exec_async is True

assert pipe.set_affinity is True

Suggested change

assert pipe.exec_pipelined is True

assert pipe.exec_async is True

assert pipe.set_affinity is True

assert pipe.exec_pipelined

assert pipe.exec_async

assert pipe.set_affinity

Again, I wanted this check to enforce the type of the attribute (to be boolean). I can't be sure that it was an initial intention of the author, but explicit comparison to True literal here made me think so. And it is an attribute of the object we define, so maybe we should test that its type does not unexpectedly change from boolean to something that is accidentally true-ish.

jantonguirao · 2022-06-17T07:27:10Z

dali/test/python/test_pipeline.py

        assert pipe.max_streams == -1
        assert pipe.prefetch_queue_depth == {"cpu_size": 3, "gpu_size": 2}
        assert pipe.cpu_queue_size == 3
        assert pipe.gpu_queue_size == 2
        assert pipe.py_num_workers == 3
        assert pipe.py_start_method == "fork"
-        assert pipe.enable_memory_stats == False
+        assert pipe.enable_memory_stats is False


Suggested change

assert pipe.enable_memory_stats is False

assert not pipe.enable_memory_stats

Same concern as above.

mzient · 2022-06-20T11:15:22Z

dali/test/python/test_dali_variable_batch_size.py

+        bboxes = [
+            np.random.random(size=test_box_shape).astype(dtype=np.float32)
+            for _ in range(batch_size)]


I think that it reads better that an opening brace on a new line is matched with a closing brace on its own line:

Suggested change

bboxes = [

np.random.random(size=test_box_shape).astype(dtype=np.float32)

for _ in range(batch_size)]

bboxes = [

np.random.random(size=test_box_shape).astype(dtype=np.float32)

for _ in range(batch_size)

]

TBH, I don't even know if the standard allows this.
This should be legal and within length limit:

Suggested change

bboxes = [

np.random.random(size=test_box_shape).astype(dtype=np.float32)

for _ in range(batch_size)]

bboxes = [np.random.random(size=test_box_shape).astype(dtype=np.float32)

for _ in range(batch_size)]

Anyway, these are just suggestions - I won't push it.

Maybe it's weird personal preference: line break on opening of list comprehension leaves a whole line for the expression inside and highlights it nicely, while there is no similar reason for doing so with the closing bracket. Contrary, it weirdly separates the whole comprehension from the block of code it rests in. Anyway, here it perfectly fits with no break, so I reverted the change.

mzient · 2022-06-20T11:16:12Z

dali/test/python/test_dali_variable_batch_size.py

+        data = [
+            np.random.randint(0, 255, size=test_data_shape, dtype=np.uint8)
+            for _ in range(batch_size)]
+        bboxes = [
+            np.random.random(size=test_box_shape).astype(dtype=np.float32)
+            for _ in range(batch_size)]
+        labels = [
+            np.random.randint(0, 255, size=test_lables_shape, dtype=np.int32)
+            for _ in range(batch_size)]


...and here, too.

mzient · 2022-06-20T11:16:26Z

dali/test/python/test_dali_variable_batch_size.py

+        bboxes = [
+            np.random.random(size=test_box_shape).astype(dtype=np.float32)
+            for _ in range(batch_size)]
+        labels = [
+            np.random.randint(0, 255, size=test_lables_shape, dtype=np.int32)
+            for _ in range(batch_size)]


...likewise...

mzient · 2022-06-20T11:20:04Z

dali/test/python/test_operator_join.py

+            [[1, 2, 3, 4],
             [5, 6, 7, 8],
-             [9,10,11,12]]))
+             [9, 10, 11, 12]]))


Can you keep column alignment?

Suggested change

[[1, 2, 3, 4],

[5, 6, 7, 8],

[9,10,11,12]]))

[9, 10, 11, 12]]))

[[1, 2, 3, 4],

[5, 6, 7, 8],

[9, 10, 11, 12]]))

mzient · 2022-06-20T12:01:51Z

dali/test/python/test_operator_readers_numpy.py

+        (fn.random.uniform(range=(0, 2), shape=(2, ), dtype=types.INT32), None,
+         fn.random.uniform(range=(7, 10), shape=(2, ),
+                           dtype=types.INT32), None, None, None, (0, 1), None),
+        (fn.random.uniform(range=(0, 2), shape=(1, ), dtype=types.INT32), None,
+         fn.random.uniform(range=(7, 10), shape=(1, ),
+                           dtype=types.INT32), None, None, None, (1, ), None),
+        (None, fn.random.uniform(range=(0.0, 0.2), shape=(1, )), None,
+         fn.random.uniform(range=(0.8, 1.0), shape=(1, )), None, None, (1, ), None),


Suggested change

(fn.random.uniform(range=(0, 2), shape=(2, ), dtype=types.INT32), None,

fn.random.uniform(range=(7, 10), shape=(2, ),

dtype=types.INT32), None, None, None, (0, 1), None),

(fn.random.uniform(range=(0, 2), shape=(1, ), dtype=types.INT32), None,

fn.random.uniform(range=(7, 10), shape=(1, ),

dtype=types.INT32), None, None, None, (1, ), None),

(None, fn.random.uniform(range=(0.0, 0.2), shape=(1, )), None,

fn.random.uniform(range=(0.8, 1.0), shape=(1, )), None, None, (1, ), None),

(fn.random.uniform(range=(0, 2), shape=(2, ), dtype=types.INT32), None,

fn.random.uniform(range=(7, 10), shape=(2, ), dtype=types.INT32),

None, None, None, (0, 1), None),

(fn.random.uniform(range=(0, 2), shape=(1, ), dtype=types.INT32), None,

fn.random.uniform(range=(7, 10), shape=(1, ), dtype=types.INT32),

None, None, None, (1, ), None),

(None, fn.random.uniform(range=(0.0, 0.2), shape=(1, )), None,

fn.random.uniform(range=(0.8, 1.0), shape=(1, )), None, None, (1, ), None),

mzient · 2022-06-20T12:03:30Z

dali/test/python/test_operator_readers_numpy.py

+def _testimpl_numpy_reader_roi_error(file_root, batch_size, ndim, dtype, device,
+                                     fortran_order=False, file_filter="*.npy", roi_start=None,
+                                     rel_roi_start=None, roi_end=None, rel_roi_end=None,
+                                     roi_shape=None, rel_roi_shape=None, roi_axes=None,
+                                     out_of_bounds_policy=None, fill_value=None):


Suggested change

def _testimpl_numpy_reader_roi_error(file_root, batch_size, ndim, dtype, device,

fortran_order=False, file_filter="*.npy", roi_start=None,

rel_roi_start=None, roi_end=None, rel_roi_end=None,

roi_shape=None, rel_roi_shape=None, roi_axes=None,

out_of_bounds_policy=None, fill_value=None):

def _testimpl_numpy_reader_roi_error(file_root, batch_size, ndim, dtype, device,

fortran_order=False, file_filter="*.npy",

roi_start=None, rel_roi_start=None,

roi_end=None, rel_roi_end=None,

roi_shape=None, rel_roi_shape=None,

roi_axes=None,

out_of_bounds_policy=None, fill_value=None):

...or just put each one in a separate line

mzient · 2022-06-20T12:05:12Z

dali/test/python/test_operator_reduce.py

@@ -47,7 +45,7 @@ def __init__(self, data_type):
        super().__init__(data_type)
        self._data = [
            [
-                np.array([ 1,  2,  3,  4], dtype=self._data_type),
+                np.array([1,  2,  3,  4], dtype=self._data_type),


Suggested change

np.array([1, 2, 3, 4], dtype=self._data_type),

np.array([ 1, 2, 3, 4], dtype=self._data_type),

Linter complains about leading space after [. I decided on aligning the remaining columns to the right while leaving the first one aligned to the left.

mzient · 2022-06-20T12:05:27Z

dali/test/python/test_operator_reduce.py

-                np.array([[ 13, 23, 22], [ 23, 21, 14]], dtype=self._data_type),
-                np.array([[ 23,  3,  6], [  7,  0, 20]], dtype=self._data_type)
+                np.array([[13, 23, 22], [23, 21, 14]], dtype=self._data_type),
+                np.array([[23,  3,  6], [7,  0, 20]], dtype=self._data_type)


Suggested change

np.array([[23, 3, 6], [7, 0, 20]], dtype=self._data_type)

np.array([[23, 3, 6], [ 7, 0, 20]], dtype=self._data_type)

same as above

mzient · 2022-06-20T12:07:22Z

dali/test/python/test_operator_reduce.py

+        np.uint8, np.int8, np.uint16, np.int16, np.uint32, np.int32,
+        np.uint64, np.int64, np.float32


when we're at it....

Suggested change

np.uint8, np.int8, np.uint16, np.int16, np.uint32, np.int32,

np.uint64, np.int64, np.float32

np.uint8, np.int8,

np.uint16, np.int16,

np.uint32, np.int32,

np.uint64, np.int64,

np.float32

mzient · 2022-06-20T12:07:50Z

dali/test/python/test_operator_reduce.py

+        np.uint8, np.int8, np.uint16, np.int16, np.uint32, np.int32,
+        np.uint64, np.int64, np.float32


Suggested change

np.uint8, np.int8, np.uint16, np.int16, np.uint32, np.int32,

np.uint64, np.int64, np.float32

np.uint8, np.int8,

np.uint16, np.int16,

np.uint32, np.int32,

np.uint64, np.int64,

np.float32

mzient · 2022-06-20T12:14:30Z

dali/test/python/test_pipeline.py

+    for reader_type in {
+            "readers.MXNet", "readers.Caffe", "readers.Caffe2", "readers.File", "readers.TFRecord",
+            "readers.Webdataset"
+    }:


mzient · 2022-06-20T12:15:19Z

dali/test/python/test_pipeline.py

+    for reader_type in {
+            "readers.MXNet", "readers.Caffe", "readers.Caffe2", "readers.File", "readers.Webdataset"
+    }:


mzient · 2022-06-20T12:16:27Z

dali/test/python/test_pipeline.py

+        out1 = as_array(out[0].at(i))
+        out2 = as_array(out[1].at(i))
+        out3 = as_array(out[2].at(i))


When using as_array, you should use out[0][i]

Suggested change

out1 = as_array(out[0].at(i))

out2 = as_array(out[1].at(i))

out3 = as_array(out[2].at(i))

out1 = as_array(out[0][i])

out2 = as_array(out[1][i])

out3 = as_array(out[2][i])

Sure, thanks!

mzient · 2022-06-20T12:16:50Z

dali/test/python/test_pipeline.py

+        out1 = as_array(out[0].at(i))
+        out2 = as_array(out[1].at(i))
+        out3 = as_array(out[2].at(i))


Suggested change

out1 = as_array(out[0].at(i))

out2 = as_array(out[1].at(i))

out3 = as_array(out[2].at(i))

out1 = as_array(out[0][i])

out2 = as_array(out[1][i])

out3 = as_array(out[2][i])

mzient · 2022-06-20T12:20:07Z

dali/test/python/test_operator_join.py

+            [[1, 2,  3,  4],
+             [5, 6,  7,  8],
+             [9, 10, 11, 12]]))


Normally, numbers are aligned to the right or to the decimal point.

Suggested change

[[1, 2, 3, 4],

[5, 6, 7, 8],

[9, 10, 11, 12]]))

[[1, 2, 3, 4],

[5, 6, 7, 8],

[9, 10, 11, 12]]))

mzient · 2022-06-20T12:20:21Z

dali/test/python/test_operator_join.py

+    ref = np.array([[1, 2,  3,  4,  13, 14, 15],
+                    [5, 6,  7,  8,  16, 17, 18],
+                    [9, 10, 11, 12, 19, 20, 21]])


Suggested change

ref = np.array([[1, 2, 3, 4, 13, 14, 15],

[5, 6, 7, 8, 16, 17, 18],

[9, 10, 11, 12, 19, 20, 21]])

ref = np.array([[1, 2, 3, 4, 13, 14, 15],

[5, 6, 7, 8, 16, 17, 18],

[9, 10, 11, 12, 19, 20, 21]])

mzient · 2022-06-20T12:23:38Z

dali/test/python/test_video_pipeline.py

+                                       sequence_length=sequence_length, shard_id=0,
+                                       num_shards=num_shards, random_shuffle=shuffle,


Suggested change

sequence_length=sequence_length, shard_id=0,

num_shards=num_shards, random_shuffle=shuffle,

sequence_length=sequence_length,

shard_id=0, num_shards=num_shards,

random_shuffle=shuffle,

...or just go with 1 argument per line

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>

stiepan · 2022-06-21T12:07:45Z

!build

dali-automaton · 2022-06-21T12:10:00Z

CI MESSAGE: [5137810]: BUILD STARTED

dali-automaton · 2022-06-21T13:21:17Z

CI MESSAGE: [5137810]: BUILD PASSED

jantonguirao assigned jantonguirao, prak-nv and mzient Jun 15, 2022

mzient reviewed Jun 15, 2022

View reviewed changes

jantonguirao reviewed Jun 17, 2022

View reviewed changes

prak-nv approved these changes Jun 20, 2022

View reviewed changes

mzient reviewed Jun 20, 2022

View reviewed changes

stiepan added 16 commits June 21, 2022 12:20

Fix linter issues in variable batch size tests

473b01a

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>

Fix linting issues in test_pipeline

e8594a4

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>

Fix lint issues in test_operator_mfcc

b856f1a

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>

Fix lint issues in test_video_pipeline.py

8ac3575

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>

Fix lint issues in test_operator_readers_numpy.py

07eed4c

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>

Fix linting issues in test_optical_flow.py

02e54fc

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>

Fix linting issues in test_backend_impl_gpu.py

ebcee42

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>

Fix linting issues in test_operator_laplacian.py

d7eb442

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>

Fix linting issues in test_operator_erase.py

818acd6

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>

Fix linting issues in test_operator_mel_filter_bank.py

6ce0379

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>

Fix lint issues in tests

d5afb81

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>

Fix laplacian tests

c980b49

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>

Apply review remarks

ad01124

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>

Review remarks

b079f4b

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>

Apply review remakrs

5f6a19b

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>

Apply review remakrs

43fcc80

Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>

stiepan force-pushed the fix_python_lint_isses_2823 branch from b6eca99 to 43fcc80 Compare June 21, 2022 11:32

mzient approved these changes Jun 21, 2022

View reviewed changes

jantonguirao approved these changes Jun 21, 2022

View reviewed changes

stiepan merged commit 8c06d53 into NVIDIA:main Jun 21, 2022

		yield raises(TypeError, expected_msg)(dali_pipe_deprecated), {"shapes": 2}, 2, tf.uint8, \
		dali_types.UINT8, 1, 2

		data = generate_data(31, 13, custom_shape_generator(3, 7, 160, 200, 80, 100, 3, 3), lo=0,
		hi=255, dtype=np.uint8)

		'batch_processing': True, 'devices': ['cpu'], 'in_types': [types.UINT8],
		'ins_ndim': [3], 'out_types': [types.UINT8], 'outs_ndim': [3],

		'batch_processing': False, 'devices': ['cpu'], 'in_types': [types.UINT8],
		'ins_ndim': [3], 'out_types': [types.UINT8], 'outs_ndim': [3],

		for (input_layout, input_shape, axis_names, anchor, shape, anchor_norm,
		shape_norm, fill_value) in rois:

		input_shape, anchor, shape, axis_names, input_layout, anchor_norm_arg, \
		shape_norm_arg, normalized_anchor, normalized_shape

		yield check_operator_erase_with_normalized_coords, device, batch_size, input_shape, anchor_arg,\
		shape_arg, axis_names, input_layout, anchor_norm_arg, shape_norm_arg, True, True

	assert tensor_list[0].__cuda_array_interface__['data'][1] is True
	assert tensor_list[0].__cuda_array_interface__['data'][1]

	assert pipe.enable_memory_stats is False
	assert not pipe.enable_memory_stats

	np.array([1, 2, 3, 4], dtype=self._data_type),
	np.array([ 1, 2, 3, 4], dtype=self._data_type),

	np.array([[23, 3, 6], [7, 0, 20]], dtype=self._data_type)
	np.array([[23, 3, 6], [ 7, 0, 20]], dtype=self._data_type)

		np.uint8, np.int8, np.uint16, np.int16, np.uint32, np.int32,
		np.uint64, np.int64, np.float32

		sequence_length=sequence_length, shard_id=0,
		num_shards=num_shards, random_shuffle=shuffle,

Fix Python formatting #3988

Fix Python formatting #3988

Conversation

stiepan commented Jun 15, 2022 • edited

Category:

Description:

Additional information:

Affected modules and functionalities:

Key points relevant for the review:

Tests:

Checklist

Documentation

DALI team only

Requirements

stiepan commented Jun 15, 2022

dali-automaton commented Jun 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dali-automaton commented Jun 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mzient Jun 15, 2022 • edited

Choose a reason for hiding this comment

mzient Jun 15, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stiepan Jun 17, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mzient Jun 20, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mzient Jun 20, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stiepan commented Jun 15, 2022 •

edited

mzient Jun 15, 2022 •

edited

mzient Jun 15, 2022 •

edited

stiepan Jun 17, 2022 •

edited

mzient Jun 20, 2022 •

edited

mzient Jun 20, 2022 •

edited