-
Notifications
You must be signed in to change notification settings - Fork 7.8k
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
【OCR Issue No.12】Modify the setuptools configuration from SETUP.py into PYPROJECT.toml #12013
Conversation
Thanks for your contribution! |
VERSION_NUMBER
Outdated
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.
加了这个文件的话,就得对 paddleocr.py 进行改造了。
可以试试,在 pyproject.toml 中用 setuptools_scm 自动生成一个 version.py , 然后让 paddleocr.py 从这里获取版本号。 可以参考一下 paddle2onnx 里的做法。
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.
.github/workflows/python-publish.yml
这个也要改
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.
如果希望setuptools_scm 生成version,则需要在ppocr/init.py中import这个version.py,打包就会采用默认生成的编号了。这样做将会彻底撇开VERSION_NUMBER。
按照pp2onnx的写法,最终应该还是在读VERSION_NUMBER的版本号。
paddleocr.py
Outdated
@@ -78,7 +78,8 @@ def _import_file(module_name, file_path, make_importable=False): | |||
] | |||
|
|||
SUPPORT_DET_MODEL = ["DB"] | |||
VERSION = "2.8.0" | |||
with open("VERSION_NUMBER", 'r') as file: |
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.
这么做还是太 tircky 了。
另外不确定,打包的时候会不会把 VERSION_NUMBER 文件打包进去。
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.
这个我打包的做法是把版本号和tag名称绑定。 指定tag时,自动触发打包action,自动读取tag号,打到包里。
可以参考我这个:
paddleocr.py
Outdated
@@ -78,7 +78,8 @@ def _import_file(module_name, file_path, make_importable=False): | |||
] | |||
|
|||
SUPPORT_DET_MODEL = ["DB"] | |||
VERSION = "2.8.0" | |||
with open("VERSION_NUMBER", 'r') as file: |
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.
ppocr/__init__.py
可不可以在这里加一个 __version__
# Copyright (c) 2019 PaddlePaddle Authors. All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import importlib.metadata as importlib_metadata
try:
__version__ = importlib_metadata.version(__package__ or __name__)
except importlib_metadata.PackageNotFoundError:
__version__ = '0.0.0'
这样 paddleocr.py 文件里就不需要VERSION变量了
"Topic :: Utilities", | ||
], | ||
) | ||
setup() |
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.
这个可以留着,用来兼容以前的在命令行下运行 python setup.py
的使用习惯。
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.
需要merge一下最新的代码
paddleocr.py
Outdated
@@ -78,7 +79,7 @@ def _import_file(module_name, file_path, make_importable=False): | |||
] | |||
|
|||
SUPPORT_DET_MODEL = ["DB"] | |||
VERSION = "2.8.0" | |||
VERSION = __version__ |
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.
这行我觉得不需要了。
只是会 break 原来有极少的人用 paddleocr.VERSION
的用法。
ppocr/__init__.py
Outdated
@@ -11,3 +11,9 @@ | |||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | |||
# See the License for the specific language governing permissions and | |||
# limitations under the License. | |||
import importlib.metadata as importlib_metadata |
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.
这个文件,我觉得不需要改动。
可以在 pyproject.toml 里配置 setuptools_scm 自动生成实现类似功能的代码。
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.
如果这个文件不需要改动,其实也不需要使用setuptools_scm
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.
我意思是不要自己实现这个功能,而是用 setuptools_scm 这样的标准化工具来实现。
pyproject.toml
Outdated
|
||
[tool.setuptools.dynamic] | ||
version = {file = "VERSION_NUMBER"} | ||
|
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.
在这里添加类似
[tool.setuptools_scm]
version_file = "pkg/_version.py"
的配置。
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, let's merge this.
Future work:
- correct the whole codebase's linting errors
- upgrade versioning mechanism
The push request changes the packaging profile from setup.py to pyproject.toml, and this push request is only used to synchronize the progress of the change.
There is currently no need to review and merge.
related projects:
#11906
refs:
https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html#configuring-setuptools-using-pyproject-toml-files