GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-47152: Convert the re module into a package by serhiy-storchaka · Pull Request #32177 · python/cpython

Conversation

@serhiy-storchaka

@serhiy-storchaka serhiy-storchaka commented Mar 29, 2022 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

https://bugs.python.org/issue47152

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@serhiy-storchaka
bpo-47152: Convert the re module into a package
ef52a13
@bedevere-bot bedevere-bot added the awaiting core review label Mar 29, 2022
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Mar 29, 2022
serhiy-storchaka added 4 commits March 29, 2022 21:19
@serhiy-storchaka
Keep old sre_* modules, but deprecate them.
a746a7d
@serhiy-storchaka
Fix sre_parse.py.
b4ec957
@serhiy-storchaka
Fix installation.
cfc39b9
@serhiy-storchaka
Keep old names of sre_* modules after moving them.
3da0b4c
arhadthedev
arhadthedev requested changes Mar 30, 2022
View reviewed changes
Comment thread Lib/re/sre_compile.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@serhiy-storchaka @arhadthedev
Update Lib/re/sre_compile.py
9abbc70
Co-authored-by: Oleg Iarygin <[email protected]>
vstinner
vstinner reviewed Apr 1, 2022
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

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.

Would it make sense to drop "sre_" prefix of module moved into the re package? re.sre_xxx now sounds redundant to me.

Overall, I like the approach!

Would it make sense to put an underscore prefix to "internal" sub-modules like re.sre_compile? For example, rename it to re._compile. asyncio, email and multiprocessing packages don't attempt to hide sub-modules. I don't know what's the best approach here.

The pip project now puts its whole code into a private pip._interal package: https://github.com/pypa/pip/tree/main/src/pip People used to abuse internals which was too easy to access. Now the "pip" package is a front-end only exposing the public API. In the pyperf project, all sub-modules are prefixed by an underscore: https://github.com/psf/pyperf/tree/main/pyperf In my experience, it was a good idea it since the internal API changed many times and I wanted to make sure that people don't use the internal API but only the public and tested API.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@vstinner

vstinner commented Apr 1, 2022

Copy link
Copy Markdown
Member

The gevent has C extensions (sub-modules) in its gevent package namespace. It might be possible to move the _sre extension as re._sre (or a different name), but the Python build system is not really designed for that. And currently, Linux distributions separate .py files and .so files in two separated directories. I suggest to not change that. It's ok to keep _sre as it is. I don't think that moving it is worth the troubles it would create.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@serhiy-storchaka

serhiy-storchaka commented Apr 1, 2022

Copy link
Copy Markdown
Member Author

It was my initial approach. Look at changes excluding 3da0b4c.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner

vstinner commented Apr 1, 2022

Copy link
Copy Markdown
Member

It was my initial approach. Look at changes excluding 3da0b4c.

If a project currently uses sre_constants, it can use from re import _constants as sre_constants, no? What is the benefits of using keeping the old names?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

serhiy-storchaka added 4 commits April 2, 2022 10:21
@serhiy-storchaka
Revert "Keep old names of sre_* modules after moving them."
d1e520a
This reverts commit 3da0b4c.
@serhiy-storchaka
Keep undescored names in the deprecated modules.
eedf86c
@serhiy-storchaka
Fix internal references.
9114da4
@serhiy-storchaka
Merge branch 'main' into re-package
226e7bb
@serhiy-storchaka
serhiy-storchaka merged commit 1be3260 into python:main Apr 2, 2022
@bedevere-bot bedevere-bot removed the awaiting core review label Apr 2, 2022
@serhiy-storchaka
serhiy-storchaka deleted the re-package branch April 2, 2022 08:35
@tiran tiran mentioned this pull request Apr 3, 2022
bpo-40280: Add --enable-wasm-dynamic-linking (GH-32253) #32253
Merged
@serhiy-storchaka serhiy-storchaka mentioned this pull request Apr 13, 2022
Reorganize the re module sources #91308
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner left review comments
@arhadthedev arhadthedev arhadthedev requested changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@serhiy-storchaka @vstinner @arhadthedev @the-knights-who-say-ni @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.