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 — gh-106078: Isolate decimal module by CharlieZhao95 · Pull Request #107287 · python/cpython

Conversation

@CharlieZhao95

@CharlieZhao95 CharlieZhao95 commented Jul 26, 2023 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Convert decimal global state to true module state.

  • Issue: Isolate the _decimal extension module #106078

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
CharlieZhao95 added 3 commits July 14, 2023 17:18
@CharlieZhao95
Convert global state to module state
f3da37a
@CharlieZhao95
Merge remote-tracking branch 'upstream/main' into decimal/convert_state
ff69ea8
@CharlieZhao95
Fix function signature
821c398
@bedevere-bot bedevere-bot mentioned this pull request Jul 26, 2023
Isolate the _decimal extension module #106078
Closed
14 tasks done
CharlieZhao95 added 2 commits July 27, 2023 10:51
@CharlieZhao95
Update comment
b137f18
@CharlieZhao95
Add NEWS
50d5243
@CharlieZhao95
CharlieZhao95 marked this pull request as ready for review July 27, 2023 06:14
@CharlieZhao95
CharlieZhao95 requested a review from rhettinger as a code owner July 27, 2023 06:14
@bedevere-bot bedevere-bot added the awaiting review label Jul 27, 2023
@CharlieZhao95

CharlieZhao95 commented Jul 27, 2023

Copy link
Copy Markdown
Contributor Author

cc: @kumaraditya303 @erlend-aasland
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
kumaraditya303
kumaraditya303 approved these changes Jul 27, 2023
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Jul 27, 2023
@kumaraditya303 kumaraditya303 added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 27, 2023
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 27, 2023
@kumaraditya303 kumaraditya303 added 🔨 test-with-buildbots Test PR w/ buildbots; report in status section extension-modules C modules in the Modules dir labels Jul 27, 2023
@bedevere-bot

bedevere-bot commented Jul 27, 2023

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @kumaraditya303 for commit 50d5243 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 27, 2023
@python python deleted a comment from bedevere-bot Jul 27, 2023
@rhettinger
rhettinger removed their request for review July 27, 2023 10:15
@kumaraditya303
kumaraditya303 merged commit a43cc3f into python:main Jul 28, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Jul 28, 2023
@AlexWaygood

AlexWaygood commented Jul 28, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Following this PR, I now get a warning printed to my terminal whenever I run python -m test test_decimal locally on my Windows machine inside my CPython clone:

>python -m test test_decimal
Running Release|x64 interpreter...
0:00:00 Run tests sequentially
0:00:00 [1/1] test_decimal
C:\Users\alexw\coding\cpython\Modules\_decimal\libmpdec\context.c:57: warning: mpd_setminalloc: ignoring request to set MPD_MINALLOC a second time

(I bisected the warning to this PR)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
CharlieZhao95
CharlieZhao95 commented Jul 29, 2023
View reviewed changes
Comment thread Modules/_decimal/_decimal.c
@@ -5857,7 +5876,7 @@ PyInit__decimal(void)
mpd_free = PyMem_Free;
mpd_setminalloc(_Py_DEC_MINALLOC);

@CharlieZhao95 CharlieZhao95 Jul 29, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

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.

Yes. The warning occures here, which is caused by the multi-phase initialization of decimal module. @AlexWaygood

This warning does not affect the module, because a static flag is used in mpd_setminalloc to ensure that MPD_MINALLOC can only be initialized once.

Maybe we can also add a static flag to the _decimal_exec to suppress this warning. But I'm not sure if it makes sense to do so (we add a static variable just to tell the compiler that we know a warning will be raised here). :)

Sorry, something went wrong.

Uh oh!

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

@sunmy2019 sunmy2019 Oct 3, 2023

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.

I think it is reasonable. Warnings are disrupting.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Eclips4 Eclips4 mentioned this pull request Jul 30, 2023
test_tools are leaked #107458
Closed
@CharlieZhao95 CharlieZhao95 mentioned this pull request Aug 1, 2023
gh-106078: Suppress the warning caused by multi-phase initialization of decimal #107524
Merged
@AlexWaygood AlexWaygood mentioned this pull request Oct 3, 2023
Build warnings - October 2023 #110266
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sunmy2019 sunmy2019 sunmy2019 left review comments
@kumaraditya303 kumaraditya303 kumaraditya303 approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

extension-modules C modules in the Modules dir

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

@CharlieZhao95 @bedevere-bot @AlexWaygood @sunmy2019 @kumaraditya303
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.