-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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.
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
optimize setup.py under conda virtual environment #48211
Conversation
你的PR提交成功,感谢你对开源项目的贡献! |
a26a578
to
941bf62
Compare
setup.py
Outdated
@@ -1405,10 +1410,31 @@ def get_setup_parameters(): | |||
) | |||
|
|||
|
|||
def check_build_dependency(): | |||
python_dependcies_module = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个列表直接从python/requirements.txt中获取吧
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
对,完全一致
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
对,和里边完全一致
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
尽量不要维护两处一模一样的内容,直接从requirements.txt中读取
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
好的,很好的建议
installed_packages = [r.decode().split('==')[0] for r in reqs.split()] | ||
for dependency in python_dependcies_module: | ||
if dependency not in installed_packages: | ||
raise RuntimeError(missing_modules.format(dependency=dependency)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
是否可以一次暴露出所有缺失模块
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
可以,我优化一下,但是最后提示阶段,还是提示用户pip install -r python/requirements,而不是提示只安装缺失的包,因为python依赖库都有版本限制,pip install -r python/requirements可以确保依赖库版本都正确
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
是的,提示出缺少哪些包
setup.py
Outdated
@@ -76,6 +76,11 @@ | |||
|
|||
IS_WINDOWS = os.name == 'nt' | |||
|
|||
missing_modules = ''' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不需要单独维护一个global string,直接写在函数中就好
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok,我改一下
f.read().splitlines() | ||
) # Specify the dependencies to install | ||
|
||
python_dependcies_module = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
利用正则会更简单一些,例如:re.sub("_|-", '', re.sub(r"==.|>=.|<=.*", '', "abc_d-e<=1.1")) 返回 abcde
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR types
others
PR changes
others
Describe
优化setup.py

当我们通过conda 创建一个新的空的环境时候,setup.py不会检查依赖,直接编译,会在cmake阶段报错缺少依赖,现在在cmake之前进行判断,并提示用户安装依赖,如缺少protobuf,则会提示,如图