bpo-1635741: Port _datetime extension module to multiphase initialization (PEP 489) by CharlieZhao95 ยท Pull Request #30522 ยท python/cpython

@CharlieZhao95

@CharlieZhao95

@the-knights-who-say-ni

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@CharlieZhao95

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@blurb-it

@shihai1991

@CharlieZhao95 Thanks for your contirbute. You can use ./python -m test test_datetime -v -R 3:3 to catch the failure of CI.

@erlend-aasland

@CharlieZhao95

@erlend-aasland

Thanks for your reply :) Indeed, it is better to convert global state to module state first. I will work on issue https://bugs.python.org/issue40077 (convert static types to heap types) and fix reference leak promblem caused by static types in this PR later.

Sounds good. But the steps needed should be approximately like this:

  1. Discuss the changes with the module maintainer/code owner (for example a topic on Discourse, or a bpo issue for this specific module). Explain the rationale and reasoning behind the change. Continue with the subsequent steps if the module maintainer approves.
  2. Lay out an implementation plan. Remember that there may be performance bottle necks when retrieving module state; care must be taken not to introduce a performance regression.
  3. Apply Argument Clinic to the modules types; it will help retrieving module state later in the process. Perhaps this is already done; I did not go to the gemba to look ๐Ÿ™‚
  4. Establish a global module state
    • Create a module state struct and add a (temporary) instance on the module stack
    • Move global variables to module state struct one by one. (This is where issue 40077 happens!)
  5. Convert global module state to module state; take care that no performance regression is introduced => benchmark
  6. Apply multi-phase initialisation

IMO, this process should be spread out over several PRs, to make it easier to review. (I try to keep the diffs of my PRs below 200 lines of code.) It is IMO of importance that all of this happens in the course of a single alpha development phase. You cannot use a beta phase for changes like this. 3.11 beta starts in about 4 months time from now, IIRC.

I suggest you take a shot at it on your own fork, so you'll be ready for when the SC has approved we can continue with these changes ๐Ÿ˜ƒ

@erlend-aasland

Oh, thanks for signing the CLA! ๐Ÿš€ ๐Ÿ˜ƒ

@erlend-aasland

@kylotan kylotan mannequin mentioned this pull request

Sep 19, 2022