Skip to content
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

Merged
merged 21 commits into from
May 24, 2024

Conversation

Liyulingyue
Copy link
Contributor

@Liyulingyue Liyulingyue commented Apr 26, 2024

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

Copy link

paddle-bot bot commented Apr 26, 2024

Thanks for your contribution!

@Liyulingyue Liyulingyue changed the title 【Don't Review】Modify the setuptools configuration from SETUP.py into PYPROJECT.toml 【OCR Issue No.12】Modify the setuptools configuration from SETUP.py into PYPROJECT.toml Apr 27, 2024
pyproject.toml Outdated Show resolved Hide resolved
@GreatV GreatV requested a review from jzhang533 April 27, 2024 10:49
pyproject.toml Outdated Show resolved Hide resolved
setup_backup.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
VERSION_NUMBER Outdated
Copy link
Collaborator

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 里的做法。

Copy link
Collaborator

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
这个也要改

Copy link
Contributor Author

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:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这么做还是太 tircky 了。
另外不确定,打包的时候会不会把 VERSION_NUMBER 文件打包进去。

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个我打包的做法是把版本号和tag名称绑定。 指定tag时,自动触发打包action,自动读取tag号,打到包里。
可以参考我这个:

https://github.com/RapidAI/RapidOCR/blob/a5a8eb30e82ae578574f2cba38aaba4269be9c00/.github/workflows/gen_whl_to_pypi_rapidocr_ort.yml#L91

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:
Copy link
Collaborator

@GreatV GreatV May 15, 2024

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()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个文件应该不需要了吧

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个可以留着,用来兼容以前的在命令行下运行 python setup.py 的使用习惯。

Copy link
Collaborator

@GreatV GreatV left a 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__
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这行我觉得不需要了。
只是会 break 原来有极少的人用 paddleocr.VERSION 的用法。

@@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个文件,我觉得不需要改动。
可以在 pyproject.toml 里配置 setuptools_scm 自动生成实现类似功能的代码。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果这个文件不需要改动,其实也不需要使用setuptools_scm

Copy link
Collaborator

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"}

Copy link
Collaborator

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"

的配置。

详情: https://github.com/pypa/setuptools_scm

paddleocr.py Outdated Show resolved Hide resolved
paddleocr.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@jzhang533 jzhang533 left a 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

@jzhang533 jzhang533 merged commit 3a66efc into PaddlePaddle:main May 24, 2024
3 of 4 checks passed
@Liyulingyue Liyulingyue deleted the setup2toml branch May 25, 2024 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants