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.

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

AssertionError in Doc/includes/mp_benchmarks.py #48699

Closed
tiran opened this issue Nov 28, 2008 · 24 comments
Closed

AssertionError in Doc/includes/mp_benchmarks.py #48699

tiran opened this issue Nov 28, 2008 · 24 comments
Labels
extension-modules C modules in the Modules dir release-blocker type-bug An unexpected behavior, bug, or error

Comments

@tiran
Copy link
Member

tiran commented Nov 28, 2008

BPO 4449
Nosy @loewis, @warsaw, @rhettinger, @amauryfa, @tiran
Files
  • mp_array.patch
  • mp_array_2.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2009-01-18.02:47:18.419>
    created_at = <Date 2008-11-28.11:17:04.259>
    labels = ['extension-modules', 'type-bug', 'release-blocker']
    title = 'AssertionError in Doc/includes/mp_benchmarks.py'
    updated_at = <Date 2009-01-18.19:46:38.898>
    user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2009-01-18.19:46:38.898>
    actor = 'jnoller'
    assignee = 'jnoller'
    closed = True
    closed_date = <Date 2009-01-18.02:47:18.419>
    closer = 'jnoller'
    components = ['Extension Modules']
    creation = <Date 2008-11-28.11:17:04.259>
    creator = 'christian.heimes'
    dependencies = []
    files = ['12204', '12205']
    hgrepos = []
    issue_num = 4449
    keywords = ['patch', 'needs review']
    message_count = 24.0
    messages = ['76525', '76526', '76530', '76531', '76532', '76544', '76656', '76725', '76742', '76773', '76774', '76784', '76785', '76788', '76790', '76792', '76793', '76794', '76795', '76796', '79623', '79759', '80072', '80073']
    nosy_count = 6.0
    nosy_names = ['loewis', 'barry', 'rhettinger', 'amaury.forgeotdarc', 'christian.heimes', 'jnoller']
    pr_nums = []
    priority = 'release blocker'
    resolution = 'fixed'
    stage = 'patch review'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue4449'
    versions = ['Python 2.6']

    @tiran
    Copy link
    Member Author

    tiran commented Nov 28, 2008

    ./python Doc/includes/mp_benchmarks.py

       ######## testing Array("i", ..., lock=False)
    
    Traceback (most recent call last):
      File "Doc/includes/mp_benchmarks.py", line 235, in <module>
        test()
      File "Doc/includes/mp_benchmarks.py", line 203, in test
        test_seqspeed(multiprocessing.Array('i', range(10), lock=False))
      File
    "/home/heimes/dev/python/release26-maint/Lib/multiprocessing/__init__.py",
    line 254, in Array
        return Array(typecode_or_type, size_or_initializer, **kwds)
      File
    "/home/heimes/dev/python/release26-maint/Lib/multiprocessing/sharedctypes.py",
    line 87, in Array
        assert hasattr(lock, 'acquire')
    AssertionError

    The assertion error occurs when using Python 2.6 and our backports to
    2.4 and 2.5.

    @tiran tiran added the stdlib Python modules in the Lib dir label Nov 28, 2008
    @tiran tiran added the type-bug An unexpected behavior, bug, or error label Nov 28, 2008
    @tiran
    Copy link
    Member Author

    tiran commented Nov 28, 2008

    The examples in 3.0 didn't work at at all because nobody did a 2to3 run
    on them. See r67417: mp_benchmarks, mp_newtypes and mp_distribution are
    still broken but the others are working properly. We should include the
    examples in our unit test suite ...

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Nov 28, 2008

    The 3.0 doc/example issue is in bpo-3256

    I plan on fixing all the doc/example issue this/next week.

    @tiran
    Copy link
    Member Author

    tiran commented Nov 28, 2008

    Are you able to fix the examples before 3.0.0 and 2.6.1 are released?
    They are scheduled for Wednesday 3rd of December.

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Nov 28, 2008

    Yes, I have a pending patch. I'll see if I can steal some time today
    to check it in.

    On Fri, Nov 28, 2008 at 9:36 AM, Christian Heimes
    <report@bugs.python.org> wrote:

    Christian Heimes <lists@cheimes.de> added the comment:

    Are you able to fix the examples before 3.0.0 and 2.6.1 are released?
    They are scheduled for Wednesday 3rd of December.


    Python tracker <report@bugs.python.org>
    <http://bugs.python.org/issue4449\>


    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Nov 28, 2008

    I guess you just 2to3'ed the examples

    @warsaw
    Copy link
    Member

    warsaw commented Nov 30, 2008

    Jesse, please apply so we can close this issue.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Dec 2, 2008

    This is a documentation bug, not a library bug, right? (unless someone
    claims that the documentation is right and the library is wrong)

    So: a) this seems to be a duplicate of bpo-3256, and b) it's "just" a
    documention bug. Not sure why this is marked as release blocker.

    @loewis loewis mannequin added docs Documentation in the Doc dir and removed stdlib Python modules in the Lib dir labels Dec 2, 2008
    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Dec 2, 2008

    This is not a doc bug - in actuality, the mp_benchmarks.py example is
    exposing an assertion error in sharedctypes.py.

    The doc-related bugs Christian and I spoke about have been fixed, however
    the main issue for this (the assertion error) is still in the code, and I
    haven't had a chance to trace it down and fix it.

    @jnoller jnoller mannequin added extension-modules C modules in the Modules dir and removed docs Documentation in the Doc dir labels Dec 2, 2008
    @warsaw
    Copy link
    Member

    warsaw commented Dec 2, 2008

    Jesse, does this affect Python 3.0? If not, then I will lower the
    priority and release 2.6.1 without it (there's always 2.6.2 :).

    If so, then we need to know if 1) it's really a release blocker; 2) what
    the ETA on a fix is. I would like to tag the 3.0 branch today.

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Dec 2, 2008

    2.6.0 shipped with the assertion error. Unfortunately, I'm tapped out at
    the day job right now, so I won't have a fix prepped and tested in time.

    @jnoller jnoller mannequin added deferred-blocker and removed release-blocker labels Dec 2, 2008
    @amauryfa
    Copy link
    Member

    amauryfa commented Dec 2, 2008

    Here is a patch nonetheless. It makes the code match the the documentation:

    http://docs.python.org/library/multiprocessing.html#multiprocessing.sharedctypes.Arra
    y
    """

    • If lock is True (the default) then a new lock object is created to synchronize
      access to the value.
    • If lock is a Lock or RLock object then that will be used to synchronize access to
      the value.
    • If lock is False then access to the returned object will not be automatically
      protected by a lock, so it will not necessarily be process-safe.
      """

    I changed multiprocessing.sharedctypes.Array and multiprocessing. sharedctypes.Array
    I had to change some tests: now "lock=None" is an error.
    + some markup fixes in the documentation.

    @tiran
    Copy link
    Member Author

    tiran commented Dec 2, 2008

    The patch looks fine. However I propose to replace the assert statement
    with a proper check that raises a meaningful exception.

    if not hasattr(lock, 'acquire'):
        raise AttributeError("'%r' has no method 'acquire'" % lock)

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Dec 2, 2008

    +1 on Amaury's patch, however I wouldn't change the assert right now - Christian's suggestion does make sense to change globally post 3.0

    Amaury, do you want to submit it?

    @warsaw
    Copy link
    Member

    warsaw commented Dec 2, 2008

    I don't much like the

    lock is True
    lock is False

    idioms much, but I don' thave a better suggestion, and it would be nice
    to fix this for the releases.

    @amauryfa
    Copy link
    Member

    amauryfa commented Dec 2, 2008

    Indeed, it seems that multiprocessing uses assertions in many places to
    handle errors in user code. This could be changed, but it is another
    task.

    I can apply the patch, but what about 2.6? it's an incompatible API
    change, unless we choose to process "lock=None" the same way as
    "lock=False".

    @rhettinger
    Copy link
    Contributor

    Does the language guarantee that True and False are singletons (to
    support the is-test for identity)? Does this API port to Jython and
    IronPython? Is it a problem that 1 cannot be substituted for True?

    @amauryfa
    Copy link
    Member

    amauryfa commented Dec 2, 2008

    The one I know is pypy: bool(x) always return one of the two prebuilt singletons
    (doubletons?).

    The docs explicitly mention "If lock is True...", "If lock is False...":
    http://docs.python.org/library/multiprocessing.html#multiprocessing.sharedctypes.Value

    I fear that testing the boolean value of the lock variable may have undesired effect; if
    even the main multiprocessing.RLock object has a _is_zero() method (that seems to return
    whether the lock is held or not), it is very possible that other implementations choose
    __nonzero__ for this.

    @warsaw
    Copy link
    Member

    warsaw commented Dec 2, 2008

    I've changed my mind based on the API change. This should be discussed
    on the mailing list and deferred until 2.6.2 at least.

    @amauryfa
    Copy link
    Member

    amauryfa commented Dec 2, 2008

    What about this other patch:

    • lock=None has the same semantics as before
    • lock=True is the same as lock=None (it failed before)
    • lock=False return the object without the "synchronized" wrap (it
      failed before)
    • no need to change previous tests

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Jan 11, 2009

    I think Amaury's patch (mp_array_2.patch) is correct. If there is no
    objection to its acceptance within the next two days, I think it should
    be applied.

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Jan 13, 2009

    I agree with Martin - if no one else gets to this before me, I should be
    able to submit it within the next day.

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Jan 18, 2009

    FYI, there was a small issue in the patch - namely the only way to get
    the
    AttributeError to raise in the Value/Array creation is to not pass in a
    lock, True/None - the latter two get to a lock/RLock while the
    former will have the acquire method. If you pass in False, you
    immediately get the RawValue() object back.

    I changed the test to reflect this:

        self.assertRaises(AttributeError, self.Value, 'i', 5, 
    lock='navalue')
        self.assertRaises(AttributeError,
                          self.Array, 'i', range(10), lock='notalock')

    @jnoller
    Copy link
    Mannequin

    jnoller mannequin commented Jan 18, 2009

    Checked into trunk in r68708 - I pinged benjamin to see how we're handling
    merges right now as this needs to go into py3k

    @jnoller jnoller mannequin closed this as completed Jan 18, 2009
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    extension-modules C modules in the Modules dir release-blocker type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants