Skip to content

[SOT][3.8] Cleanup eval_frame python3.9- code - #72868

Merged
SigureMo merged 2 commits into
PaddlePaddle:developfrom
gouzil:py38/remove_eval_frame
May 22, 2025
Merged

SigureMo merged 2 commits into
PaddlePaddle:developfrom
gouzil:py38/remove_eval_frame

Conversation

@gouzil

@gouzil gouzil commented May 22, 2025

Copy link
Copy Markdown
Collaborator

PR Category

Execute Infrastructure

PR Types

Deprecations

Description

  • 清理 eval_frame python3.9- 相关代码

@gouzil
gouzil requested review from SigureMo and zrr1999 as code owners May 22, 2025 04:26
@paddle-bot

paddle-bot Bot commented May 22, 2025

Copy link
Copy Markdown

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册。
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@paddle-bot paddle-bot Bot added the contributor External developers label May 22, 2025
@gouzil
gouzil requested review from Copilot and removed request for SigureMo and zrr1999 May 22, 2025 04:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR cleans up legacy Python eval_frame code by removing support for Python versions older than 3.9. It deprecates outdated conditions and macros while simplifying eval_frame callback logic.

  • Update the test to require Python 3.9–3.13
  • Remove Python 3.8-specific macros and fallback code
  • Simplify version-specific branches in eval_frame implementation

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
test/dygraph_to_static/test_eval_frame.py Update test condition and comment to reflect new Python version support
paddle/fluid/pybind/sot/macros.h Remove obsolete Python 3.8 definitions and macros
paddle/fluid/pybind/sot/guards.cc Remove fallback implementation for PyObject_CallOneArg for Python below 3.9
paddle/fluid/pybind/sot/eval_frame.c Eliminate legacy conditional branches for Python versions earlier than 3.9
Comments suppressed due to low confidence (2)

paddle/fluid/pybind/sot/eval_frame.c:63

  • Add a comment noting that the alternative branch for Python versions below 3.9 has been removed to simplify the eval_frame logic.
return _PyEval_EvalFrameDefault(tstate, frame, throw_flag);

paddle/fluid/pybind/sot/guards.cc:23

  • Confirm that the removal of the fallback for PyObject_CallOneArg is intentional and documented elsewhere, given the deprecation of pre-Python 3.9 support.
#if !defined(PyObject_CallOneArg) && !PY_3_9_PLUS

def test_eval_frame(self):
if not (sys.version_info >= (3, 8) and sys.version_info < (3, 14)):
# skip test_eval_frame, current only support 3.8 - 3.13
if not (sys.version_info >= (3, 9) and sys.version_info < (3, 14)):

Copilot AI May 22, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] Consider updating the inline comment to clearly state that only Python 3.9 to 3.13 are supported. A revision for clarity (eg. 'supported: Python 3.9 to 3.13') could benefit future readers.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

这个还是有点用的啊

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

不对啊,明明改了啊,为啥还会 comment

Comment thread paddle/fluid/pybind/sot/eval_frame.c Outdated
}

#if PY_3_9_PLUS
static PyObject *custom_eval_frame_shim(PyThreadState *tstate,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

这个函数是不是可以删掉了?再把 _custom_eval_frame_shim rename 成 custom_eval_frame_shim

@SigureMo SigureMo mentioned this pull request May 22, 2025
12 of 21 tasks

@SigureMo SigureMo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTMeow 🐾

@SigureMo SigureMo changed the title [3.8] clean eval_frame python3.9- code [3.8] Cleanup eval_frame python3.9- code May 22, 2025
@SigureMo SigureMo changed the title [3.8] Cleanup eval_frame python3.9- code [SOT][3.8] Cleanup eval_frame python3.9- code May 22, 2025
@SigureMo
SigureMo merged commit 8b676a6 into PaddlePaddle:develop May 22, 2025
@gouzil
gouzil deleted the py38/remove_eval_frame branch April 18, 2026 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor External developers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants