Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[skip issue]Small changes in importing process of multiprocessing package #6856

Merged
merged 5 commits into from Jul 11, 2018
Merged

[skip issue]Small changes in importing process of multiprocessing package #6856

merged 5 commits into from Jul 11, 2018

Conversation

bombs-kim
Copy link
Contributor

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

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.

@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
@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
Copy link
Contributor Author

bombs-kim commented May 23, 2018 •
edited

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

@1st1 1st1 requested review from applio and removed request for 1st1 May 23, 2018 05:06
@1st1
Copy link
Member

1st1 commented May 23, 2018

Added Davin as a reviewer as multiprocessing is his territory.

@serhiy-storchaka
Copy link
Member

Please add tests for __all__. The test.support.check__all__ helper can be used for this.

@bombs-kim
Copy link
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

@serhiy-storchaka
Copy link
Member

Just look how this helper is used in other tests.

@bombs-kim
Copy link
Contributor Author

bombs-kim commented Jun 1, 2018 •
edited

@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.

@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
Copy link
Contributor Author

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

methane
methane reviewed Jun 28, 2018
View reviewed changes
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('_')]
Copy link
Member

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('_')?

Copy link
Contributor Author

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🙂

Lib/multiprocessing/context.py Outdated
@@ -5,7 +5,7 @@
from . import process
from . import reduction

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

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?

Copy link
Contributor Author

@bombs-kim bombs-kim Jun 28, 2018 •
edited

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

Copy link
Contributor Author

@bombs-kim bombs-kim Jul 3, 2018 •
edited

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.

@methane methane merged commit c40278e into python:master 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

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.

None yet
6 participants
@bombs-kim @the-knights-who-say-ni @1st1 @serhiy-storchaka @methane @bedevere-bot