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 — [skip issue]Small changes in importing process of multiprocessing package by bombs-kim · Pull Request #6856 · python/cpython

Conversation

@bombs-kim

@bombs-kim bombs-kim commented May 15, 2018 •
edited by methane
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

It seems that the way multiprocessing package set __all__ is overly complex. And __all__ in context.py is not used in __init__.py so I guess the comment there is misleading.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Derek Kim added 2 commits May 15, 2018 18:55
Derek Kim
Changed unnecessarily complex way of updating of __all__ and globals …
02c298d 
…in __init__.py
Derek Kim
Maybe a misguiding comment
eb07cb5
@bombs-kim
bombs-kim requested a review from 1st1 as a code owner May 15, 2018 10:08
@the-knights-who-say-ni

This comment has been minimized.

Sign in to view
@the-knights-who-say-ni the-knights-who-say-ni added the CLA not signed label May 15, 2018
@bedevere-bot bedevere-bot added the awaiting review label May 15, 2018
@bombs-kim bombs-kim changed the title Small changes in importing mechanism of multiprocessing library Small changes in importing process of multiprocessing package May 15, 2018
@bombs-kim

This comment has been minimized.

Sign in to view
@bombs-kim

bombs-kim commented May 23, 2018 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

@1st1 I've submitted PSF form. Maybe some errors has happened. Could you please check this out and remove CLA not sign tag?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@1st1 1st1 removed the CLA not signed label May 23, 2018
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label May 23, 2018
@1st1
1st1 requested review from applio and removed request for 1st1 May 23, 2018 05:06
@1st1

1st1 commented May 23, 2018

Copy link
Copy Markdown
Member

Added Davin as a reviewer as multiprocessing is his territory.
bombs-kim reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka

serhiy-storchaka commented May 23, 2018

Copy link
Copy Markdown
Member

Please add tests for __all__. The test.support.check__all__ helper can be used for this.
bombs-kim reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bombs-kim

bombs-kim commented May 24, 2018

Copy link
Copy Markdown
Contributor Author

@serhiy-storchaka Thank you so much. I will upload that as early as I can, but it can take some time because I'm new to test
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka

serhiy-storchaka commented May 24, 2018

Copy link
Copy Markdown
Member

Just look how this helper is used in other tests.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Derek Kim
Added a trivial test for __all__
25335c3
@bombs-kim

bombs-kim commented Jun 1, 2018 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

@serhiy-storchaka Sorry for being late. I've added a test case but I'm not sure this is what you intended. Public objects in multiprocessing package are bound methods of multiprocessing.context._default_context object and they don't have __module__ attributes and they are not types.ModuleType, either. So I think using check__all__ is pretty pointless here because you cannot make expected automatically in that function.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bombs-kim bombs-kim changed the title Small changes in importing process of multiprocessing package [skip issue]Small changes in importing process of multiprocessing package Jun 15, 2018
@bombs-kim

bombs-kim commented Jun 15, 2018

Copy link
Copy Markdown
Contributor Author

@applio I know it's a trivial contribution, but I want to know the progress of PR.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
methane
methane reviewed Jun 28, 2018
View reviewed changes
Comment thread Lib/multiprocessing/__init__.py Outdated
globals().update((name, getattr(context._default_context, name))
for name in context._default_context.__all__)
__all__ = context._default_context.__all__
__all__ = [x for x in dir(context._default_context) if not x[0].startswith('_')]

@methane methane Jun 28, 2018

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.

Shouldn't x[0].startswith('_') be x.startswith('_')?

Sorry, something went wrong.

Uh oh!

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

@bombs-kim bombs-kim Jun 28, 2018

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, you are right. Thank you🙂

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/multiprocessing/context.py Outdated
from . import reduction

__all__ = [] # things are copied from here to __init__.py
__all__ = []

@methane methane Jun 28, 2018

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.

Could we use __all__ = ()? Or __all__ is modified later?

Sorry, something went wrong.

Uh oh!

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

@bombs-kim bombs-kim Jun 28, 2018 •
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.

I don't think it's modified. I will run the test and tell you the result

Sorry, something went wrong.

Uh oh!

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

@bombs-kim bombs-kim Jul 3, 2018 •
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.

@methane I updated things that you pointed out. It passed all tests. Thanks.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Derek Kim added 2 commits July 3, 2018 17:43
Derek Kim
Trivial error fixed
ef222ac
Derek Kim
__all__ is not supposed to be modified
2e617a2
@methane methane added skip issue skip news labels Jul 11, 2018
@methane
methane merged commit c40278e into python:master Jul 11, 2018
@bedevere-bot bedevere-bot removed the awaiting review label Jul 11, 2018
CuriousLearner added a commit to CuriousLearner/cpython that referenced this pull request Jul 14, 2018
@CuriousLearner
Merge branch 'master' into bitwise_docs
11c1706 
* master: (2633 commits)
  bpo-34087: Fix buffer overflow in int(s) and similar functions (pythonGH-8274)
  bpo-34108: Fix double carriage return in 2to3 on Windows (python#8271)
  bpo-4260: Document that ctypes.xFUNCTYPE are decorators (pythonGH-7924)
  bpo-33723: Fix test_time.test_thread_time() (pythonGH-8267)
  bpo-33967: Remove use of deprecated assertRaisesRegexp() (pythonGH-8261)
  bpo-34080: Fix a memory leak in the compiler. (pythonGH-8222)
  Enable GUI testing on Travis Linux builds via Xvfb (pythonGH-7887)
  bpo-23927: Make getargs.c skipitem() skipping 'w*'. (pythonGH-8192)
  bpo-33648: Remove PY_WARN_ON_C_LOCALE (pythonGH-7114)
  bpo-34092, test_logging: increase SMTPHandlerTest timeout (pythonGH-8245)
  Simplify __all__ in multiprocessing (pythonGH-6856)
  bpo-34083: Update dict order in Functional HOWTO (pythonGH-8230)
  Doc: Point to Simple statements section instead of PEP (pythonGH-8238)
  bpo-29442: Replace optparse with argparse in setup.py (pythonGH-139)
  bpo-33597: Add What's New for PyGC_Head (pythonGH-8236)
  Dataclasses: Fix example on 30.6.8, add method should receive a list rather than an integer. (pythonGH-8038)
  Fix documentation for input and output tutorial (pythonGH-8231)
  bpo-34009: Expand on platform support changes (pythonGH-8022)
  Factor-out two substantially identical code blocks. (pythonGH-8219)
  bpo-34031: fix incorrect usage of self.fail in two tests (pythonGH-8091)
  ...
CuriousLearner added a commit to CuriousLearner/cpython that referenced this pull request Jul 15, 2018
@CuriousLearner
Merge branch 'master' into fix-issue8525
fdc3a3d 
* master: (1159 commits)
  bpo-34087: Fix buffer overflow in int(s) and similar functions (pythonGH-8274)
  bpo-34108: Fix double carriage return in 2to3 on Windows (python#8271)
  bpo-4260: Document that ctypes.xFUNCTYPE are decorators (pythonGH-7924)
  bpo-33723: Fix test_time.test_thread_time() (pythonGH-8267)
  bpo-33967: Remove use of deprecated assertRaisesRegexp() (pythonGH-8261)
  bpo-34080: Fix a memory leak in the compiler. (pythonGH-8222)
  Enable GUI testing on Travis Linux builds via Xvfb (pythonGH-7887)
  bpo-23927: Make getargs.c skipitem() skipping 'w*'. (pythonGH-8192)
  bpo-33648: Remove PY_WARN_ON_C_LOCALE (pythonGH-7114)
  bpo-34092, test_logging: increase SMTPHandlerTest timeout (pythonGH-8245)
  Simplify __all__ in multiprocessing (pythonGH-6856)
  bpo-34083: Update dict order in Functional HOWTO (pythonGH-8230)
  Doc: Point to Simple statements section instead of PEP (pythonGH-8238)
  bpo-29442: Replace optparse with argparse in setup.py (pythonGH-139)
  bpo-33597: Add What's New for PyGC_Head (pythonGH-8236)
  Dataclasses: Fix example on 30.6.8, add method should receive a list rather than an integer. (pythonGH-8038)
  Fix documentation for input and output tutorial (pythonGH-8231)
  bpo-34009: Expand on platform support changes (pythonGH-8022)
  Factor-out two substantially identical code blocks. (pythonGH-8219)
  bpo-34031: fix incorrect usage of self.fail in two tests (pythonGH-8091)
  ...
CuriousLearner added a commit to CuriousLearner/cpython that referenced this pull request Jul 21, 2018
@CuriousLearner
Merge branch 'master' of github.com:CuriousLearner/cpython into fix-i…
7b039a0 
…ssue-33014

* 'master' of github.com:CuriousLearner/cpython: (722 commits)
  bpo-34087: Fix buffer overflow in int(s) and similar functions (pythonGH-8274)
  bpo-34108: Fix double carriage return in 2to3 on Windows (python#8271)
  bpo-4260: Document that ctypes.xFUNCTYPE are decorators (pythonGH-7924)
  bpo-33723: Fix test_time.test_thread_time() (pythonGH-8267)
  bpo-33967: Remove use of deprecated assertRaisesRegexp() (pythonGH-8261)
  bpo-34080: Fix a memory leak in the compiler. (pythonGH-8222)
  Enable GUI testing on Travis Linux builds via Xvfb (pythonGH-7887)
  bpo-23927: Make getargs.c skipitem() skipping 'w*'. (pythonGH-8192)
  bpo-33648: Remove PY_WARN_ON_C_LOCALE (pythonGH-7114)
  bpo-34092, test_logging: increase SMTPHandlerTest timeout (pythonGH-8245)
  Simplify __all__ in multiprocessing (pythonGH-6856)
  bpo-34083: Update dict order in Functional HOWTO (pythonGH-8230)
  Doc: Point to Simple statements section instead of PEP (pythonGH-8238)
  bpo-29442: Replace optparse with argparse in setup.py (pythonGH-139)
  bpo-33597: Add What's New for PyGC_Head (pythonGH-8236)
  Dataclasses: Fix example on 30.6.8, add method should receive a list rather than an integer. (pythonGH-8038)
  Fix documentation for input and output tutorial (pythonGH-8231)
  bpo-34009: Expand on platform support changes (pythonGH-8022)
  Factor-out two substantially identical code blocks. (pythonGH-8219)
  bpo-34031: fix incorrect usage of self.fail in two tests (pythonGH-8091)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@methane methane methane left review comments
@applio applio Awaiting requested review from applio

Uh oh!

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

Assignees

No one assigned

Labels

skip issue skip news

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.

6 participants

@bombs-kim @the-knights-who-say-ni @1st1 @serhiy-storchaka @methane @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.