Skip to content

Commit ba7e2a9

Browse files
authored
remove the tricks for paddle.fluid.layers.ops.func (#33731)
* refactor check_pr_approval, allow using github login-id 2. remove the tricks for paddle.fluid.layers.ops.func * add testcases * simplify the test data, and added to file diff approvals * remove a approver * test_print_signatrues runs on a simple pipeline, no paddle installed * testcases for print_signatrures and sampcd . python3 only. * remove unused import directives * remove unused import directives
1 parent dfbfbd0 commit ba7e2a9

8 files changed

+164
-63
lines changed

tools/check_api_approvals.sh

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,7 @@ function add_failed(){
3838

3939

4040
api_spec_diff=`python ${PADDLE_ROOT}/tools/diff_api.py ${PADDLE_ROOT}/paddle/fluid/API_DEV.spec.api ${PADDLE_ROOT}/paddle/fluid/API_PR.spec.api`
41-
ops_func_in_diff=$(echo ${api_spec_diff} | grep '\bpaddle\.fluid\.layers\.ops\.func\b')
42-
linenum=$(echo ${api_spec_diff} | wc -l | sed 's/[[:space:]]//g')
43-
if [ "${linenum}" = "3" -a "${ops_func_in_diff}" != "" ] ; then
44-
echo "skip paddle.fluid.layers.ops.func"
45-
elif [ "$api_spec_diff" != "" ]; then
41+
if [ "$api_spec_diff" != "" ]; then
4642
echo_line="You must have one RD (XiaoguangHu01 or lanxianghit) approval for API change.\n"
4743
echo_line="${echo_line} and one TPM approval for API change: \n"
4844
echo_line="${echo_line} jzhang533/ZhangJun, dingjiaweiww/DingJiaWei, Heeenrrry/LiKunLun, TCChenlong/ChenLong for general APIs\n"
@@ -54,10 +50,7 @@ elif [ "$api_spec_diff" != "" ]; then
5450
fi
5551

5652
api_doc_spec_diff=`python ${PADDLE_ROOT}/tools/diff_api.py ${PADDLE_ROOT}/paddle/fluid/API_DEV.spec.doc ${PADDLE_ROOT}/paddle/fluid/API_PR.spec.doc`
57-
linenum=$(echo ${api_doc_spec_diff} | wc -l | sed 's/[[:space:]]//g')
58-
if [ "${linenum}" = "3" -a "${ops_func_in_diff}" != "" ] ; then
59-
echo "skip paddle.fluid.layers.ops.func for doc diff"
60-
elif [ "$api_doc_spec_diff" != "" ]; then
53+
if [ "$api_doc_spec_diff" != "" ]; then
6154
echo_line="You must have one TPM approval for API documents change: \n"
6255
echo_line="${echo_line} jzhang533/ZhangJun, dingjiaweiww/DingJiaWei, Heeenrrry/LiKunLun, TCChenlong/ChenLong for general API docs\n"
6356
echo_line="${echo_line} PangHua/XiangHui for distributed related API docs\n"
@@ -76,8 +69,8 @@ fi
7669

7770
op_type_spec_diff=`python ${PADDLE_ROOT}/tools/check_op_register_type.py ${PADDLE_ROOT}/paddle/fluid/OP_TYPE_DEV.spec ${PADDLE_ROOT}/paddle/fluid/OP_TYPE_PR.spec`
7871
if [ "$op_type_spec_diff" != "" ]; then
79-
echo_line="You must have one RD (Aurelius84 (Recommend) or liym27 or zhhsplendid)approval for the data_type registration of new operator. More data_type of new operator should be registered in your PR. Please make sure that both float/double (or int/int64_t) have been registered.\n For more details, please click [https://github.com/PaddlePaddle/Paddle/wiki/Data-types-of-generic-Op-must-be-fully-registered].\n"
80-
check_approval 1 9j301846 33742067 7913861
72+
echo_line="You must have one RD (Aurelius84 (Recommend) or zhhsplendid)approval for the data_type registration of new operator. More data_type of new operator should be registered in your PR. Please make sure that both float/double (or int/int64_t) have been registered.\n For more details, please click [https://github.com/PaddlePaddle/Paddle/wiki/Data-types-of-generic-Op-must-be-fully-registered].\n"
73+
check_approval 1 9301846 7913861
8174
fi
8275

8376
op_desc_diff=`python ${PADDLE_ROOT}/tools/check_op_desc.py ${PADDLE_ROOT}/paddle/fluid/OP_DESC_DEV.spec ${PADDLE_ROOT}/paddle/fluid/OP_DESC_PR.spec`
@@ -100,12 +93,9 @@ if [ -n "${echo_list}" ];then
10093
echo "There are ${failed_num} approved errors."
10194
echo "****************"
10295

103-
# L40 L48 L62 has fetch the result out.
104-
if [ "${api_spec_diff}" != "" ] ; then
105-
echo "api_spec_diff: ${api_spec_diff}"
106-
fi
107-
if [ "${api_doc_spec_diff}" != "" ] ; then
108-
echo "api_doc_spec_diff: ${api_doc_spec_diff}"
96+
# L40 L48 L62 has fetch the result out, but there are splitted.
97+
if [ "${api_spec_diff}" != "" -o "${api_doc_spec_diff}" != "" ] ; then
98+
python ${PADDLE_ROOT}/tools/diff_api.py ${PADDLE_ROOT}/paddle/fluid/API_DEV.spec ${PADDLE_ROOT}/paddle/fluid/API_PR.spec
10999
fi
110100
if [ "${op_type_spec_diff}" != "" ] ; then
111101
echo "op_type_spec_diff: ${op_type_spec_diff}"

tools/check_file_diff_approvals.sh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ API_FILES=("CMakeLists.txt"
5454
"python/paddle/fluid/tests/unittests/white_list/no_grad_set_white_list.py"
5555
"tools/print_signatures.py"
5656
"tools/sampcd_processor.py"
57+
"tools/check_pr_approval.py"
5758
"paddle/scripts/paddle_build.bat"
5859
"tools/windows/run_unittests.sh"
5960
"tools/parallel_UT_rule.py"
@@ -146,6 +147,9 @@ for API_FILE in ${API_FILES[*]}; do
146147
elif [ "${API_FILE}" == "tools/print_signatures.py" ];then
147148
echo_line="test_print_signatures.py will be executed for changed print_signatures.py.\n"
148149
run_tools_test test_print_signatures.py
150+
elif [ "${API_FILE}" == "tools/checkout_pr_approval.py" ];then
151+
echo_line="test_checkout_pr_approval.py will be executed for changed checkout_pr_approval.py.\n"
152+
run_tools_test test_checkout_pr_approval.py
149153
elif [ "${API_FILE}" == "python/paddle/distributed/fleet/__init__.py" ]; then
150154
echo_line="You must have (fuyinno4 (Recommend), raindrops2sea) approval for ${API_FILE} changes"
151155
check_approval 1 35824027 38231817

tools/check_pr_approval.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
from __future__ import print_function
1615
import sys
1716
import json
1817

@@ -24,17 +23,24 @@ def check_approval(count, required_reviewers):
2423
json_resp = json.loads(json_buff)
2524
approves = 0
2625
approved_user_ids = []
26+
approved_user_logins = set()
2727
for review in json_resp:
2828
if review["state"] == "APPROVED":
2929
approves += 1
3030
approved_user_ids.append(review["user"]["id"])
31+
approved_user_logins.add(review["user"]["login"])
3132

3233
# convert to int
3334
required_reviewers_int = set()
35+
required_reviewers_login = set()
3436
for rr in required_reviewers:
35-
required_reviewers_int.add(int(rr))
37+
if rr.isdigit():
38+
required_reviewers_int.add(int(rr))
39+
else:
40+
required_reviewers_login.add(rr)
3641

37-
if len(set(approved_user_ids) & required_reviewers_int) >= count:
42+
if len(set(approved_user_ids) & required_reviewers_int) + len(
43+
approved_user_logins & required_reviewers_login) >= count:
3844
print("TRUE")
3945
else:
4046
print("FALSE")

tools/print_signatures.py

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,14 @@
1717
Usage:
1818
./print_signature "paddle.fluid" > signature.txt
1919
"""
20-
from __future__ import print_function
2120

22-
import importlib
2321
import inspect
2422
import collections
2523
import sys
26-
import pydoc
2724
import hashlib
28-
import platform
29-
import functools
3025
import pkgutil
3126
import logging
3227
import argparse
33-
import paddle
3428

3529
member_dict = collections.OrderedDict()
3630

@@ -80,11 +74,7 @@ def is_primitive(instance):
8074

8175
ErrorSet = set()
8276
IdSet = set()
83-
skiplist = [
84-
'paddle.vision.datasets.DatasetFolderImageFolder',
85-
'paddle.truncdigamma',
86-
'paddle.fluid.layers.ops.func',
87-
]
77+
skiplist = []
8878

8979

9080
def visit_all_module(mod):
@@ -140,6 +130,7 @@ def get_all_api(root_path='paddle', attr="__all__"):
140130
"""
141131
walk through the paddle package to collect all the apis.
142132
"""
133+
import paddle
143134
global api_info_dict
144135
api_counter = 0
145136
for filefinder, name, ispkg in pkgutil.walk_packages(
@@ -229,6 +220,7 @@ def process_module(m, attr="__all__"):
229220

230221

231222
def get_all_api_from_modulelist():
223+
import paddle
232224
modulelist = [paddle]
233225
for m in modulelist:
234226
visit_all_module(m)

tools/sampcd_processor.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import multiprocessing
2828
import platform
2929
import inspect
30-
import json
3130
import argparse
3231
import shutil
3332
import re

tools/test_check_pr_approval.py

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
#! /usr/bin/env python
2+
3+
# Copyright (c) 2021 PaddlePaddle Authors. All Rights Reserved.
4+
#
5+
# Licensed under the Apache License, Version 2.0 (the "License");
6+
# you may not use this file except in compliance with the License.
7+
# You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
"""
17+
TestCases for check_pr_approval.py
18+
"""
19+
import unittest
20+
import subprocess
21+
import sys
22+
23+
24+
class Test_check_approval(unittest.TestCase):
25+
def setUp(self):
26+
self.codeset = 'UTF-8'
27+
# only key info in it
28+
self.jsonstr = """
29+
[
30+
{
31+
"id": 688077074,
32+
"node_id": "MDE3OlB1bGxSZXF1ZXN0UmV2aWV3Njg4MDc3MDc0",
33+
"user": {
34+
"login": "wadefelix",
35+
"id": 1306724,
36+
"type": "User",
37+
"site_admin": false
38+
},
39+
"body": "",
40+
"state": "COMMENTED",
41+
"author_association": "CONTRIBUTOR"
42+
},
43+
{
44+
"id": 688092580,
45+
"node_id": "MDE3OlB1bGxSZXF1ZXN0UmV2aWV3Njg4MDkyNTgw",
46+
"user": {
47+
"login": "MingMingShangTian",
48+
"id": 13469016,
49+
"type": "User",
50+
"site_admin": false
51+
},
52+
"body": "LGTM",
53+
"state": "APPROVED",
54+
"author_association": "CONTRIBUTOR"
55+
},
56+
{
57+
"id": 689175539,
58+
"node_id": "MDE3OlB1bGxSZXF1ZXN0UmV2aWV3Njg5MTc1NTM5",
59+
"user": {
60+
"login": "pangyoki",
61+
"id": 26408901,
62+
"type": "User",
63+
"site_admin": false
64+
},
65+
"body": "LGTM",
66+
"state": "APPROVED",
67+
"author_association": "CONTRIBUTOR"
68+
}
69+
]
70+
""".encode(self.codeset)
71+
72+
def test_ids(self):
73+
cmd = [sys.executable, 'check_pr_approval.py', '1', '26408901']
74+
subprc = subprocess.Popen(
75+
cmd,
76+
stdin=subprocess.PIPE,
77+
stdout=subprocess.PIPE,
78+
stderr=subprocess.PIPE)
79+
output, error = subprc.communicate(input=self.jsonstr)
80+
self.assertEqual('TRUE', output.decode(self.codeset).rstrip())
81+
82+
def test_logins(self):
83+
cmd = [sys.executable, 'check_pr_approval.py', '1', 'pangyoki']
84+
subprc = subprocess.Popen(
85+
cmd,
86+
stdin=subprocess.PIPE,
87+
stdout=subprocess.PIPE,
88+
stderr=subprocess.PIPE)
89+
output, error = subprc.communicate(input=self.jsonstr)
90+
self.assertEqual('TRUE', output.decode(self.codeset).rstrip())
91+
92+
def test_ids_and_logins(self):
93+
cmd = [
94+
sys.executable, 'check_pr_approval.py', '2', 'pangyoki', '13469016'
95+
]
96+
subprc = subprocess.Popen(
97+
cmd,
98+
stdin=subprocess.PIPE,
99+
stdout=subprocess.PIPE,
100+
stderr=subprocess.PIPE)
101+
output, error = subprc.communicate(input=self.jsonstr)
102+
#self.assertEqual('', error.rstrip())
103+
self.assertEqual('TRUE', output.decode(self.codeset).rstrip())
104+
105+
def test_check_with_required_reviewer_not_approved(self):
106+
cmd = [
107+
sys.executable, 'check_pr_approval.py', '2', 'wadefelix',
108+
' 13469016'
109+
]
110+
subprc = subprocess.Popen(
111+
cmd,
112+
stdin=subprocess.PIPE,
113+
stdout=subprocess.PIPE,
114+
stderr=subprocess.PIPE)
115+
output, error = subprc.communicate(input=self.jsonstr)
116+
self.assertEqual('FALSE', output.decode(self.codeset).rstrip())
117+
118+
119+
if __name__ == '__main__':
120+
unittest.main()

tools/test_print_signatures.py

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,9 @@
2323
"""
2424
import unittest
2525
import hashlib
26-
import inspect
2726
import functools
2827
from print_signatures import md5
29-
from print_signatures import get_functools_partial_spec
30-
from print_signatures import format_spec
31-
from print_signatures import queue_dict
32-
from print_signatures import member_dict
28+
from print_signatures import is_primitive
3329

3430

3531
def func_example(param_a, param_b):
@@ -65,30 +61,27 @@ def test_md5(self):
6561
digest = algo.hexdigest()
6662
self.assertEqual(digest, md5(func_example.__doc__))
6763

68-
def test_get_functools_partial_spec(self):
69-
partailed_func = functools.partial(func_example, 1)
70-
# args = inspect.getargspec(partailed_func)
71-
self.assertEqual('func_example(args=(1,), keywords={})',
72-
get_functools_partial_spec(partailed_func))
7364

74-
75-
class Test_format_spec(unittest.TestCase):
76-
def test_normal_func_spec(self):
77-
args = inspect.getargspec(func_example)
78-
self.assertEqual(
79-
'''ArgSpec(args=['param_a', 'param_b'], varargs=None, keywords=None, defaults=None)''',
80-
format_spec(args))
81-
82-
def test_func_spec_with_partialedfunc_as_param_default(self):
83-
# but there is no function belongs to this type in API_DEV.spec
84-
args = inspect.getargspec(func_example_2)
85-
self.assertEqual(
86-
'''ArgSpec(args=['func'], varargs=None, keywords=None, defaults=('func_example(args=(1,), keywords={})',))''',
87-
format_spec(args))
88-
89-
90-
class Test_queue_dict(unittest.TestCase):
91-
pass
65+
class Test_is_primitive(unittest.TestCase):
66+
def test_single(self):
67+
self.assertTrue(is_primitive(2))
68+
self.assertTrue(is_primitive(2.1))
69+
self.assertTrue(is_primitive("2.1.1"))
70+
self.assertFalse(
71+
is_primitive("hello paddle".encode('UTF-8'))) # True for python2
72+
self.assertFalse(is_primitive(1j))
73+
self.assertTrue(is_primitive(True))
74+
75+
def test_collection(self):
76+
self.assertTrue(is_primitive([]))
77+
self.assertTrue(is_primitive(tuple()))
78+
self.assertTrue(is_primitive(set()))
79+
self.assertTrue(is_primitive([1, 2]))
80+
self.assertTrue(is_primitive((1.1, 2.2)))
81+
self.assertTrue(is_primitive(set([1, 2.3])))
82+
self.assertFalse(is_primitive(range(3))) # True for python2
83+
self.assertFalse(is_primitive({}))
84+
self.assertFalse(is_primitive([1, 1j]))
9285

9386

9487
if __name__ == '__main__':

tools/test_sampcd_processor.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,7 @@
1616

1717
import unittest
1818
import os
19-
import tempfile
2019
import shutil
21-
import sys
22-
import importlib
2320
import re
2421
import sampcd_processor
2522
from sampcd_processor import find_all

0 commit comments

Comments
 (0)