Divmod

Merge lp://qastaging/~divmod-dev/divmod.org/829879-pypy-finalization-semantics into lp://qastaging/divmod.org

  1. 829879-pypy-finalization-semantics
  2. Merge into trunk
Proposed by Tristan Seligmann
Status: Superseded
Proposed branch: lp://qastaging/~divmod-dev/divmod.org/829879-pypy-finalization-semantics
Merge into: lp://qastaging/divmod.org
Diff against target: 153 lines (+38/-32)
3 files modified
Axiom/axiom/_fincache.py (+32/-28)
Axiom/axiom/store.py (+5/-3)
Axiom/axiom/test/test_reference.py (+1/-1)
To merge this branch: bzr merge lp://qastaging/~divmod-dev/divmod.org/829879-pypy-finalization-semantics
Reviewer Review Type Date Requested Status
Laurens Van Houtven Pending
Review via email: mp+172946@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2011-08-20.

This proposal has been superseded by a proposal from 2013-07-04.

Description of the change

Resubmitting without the "optional-pycrypto" dependency, as this is only relevant to Mantissa.

To post a comment you must log in.
Revision history for this message
Laurens Van Houtven (lvh) wrote : Posted in a previous version of this proposal

LGTM, and makes my exception go away:

https://gist.github.com/lvh/9059ae79bc3ba7b10f73

Some notes:

- this doesn't appear to have any tests.
- is the API sufficiently internal remove the "has" method?

review: Needs Fixing
Revision history for this message
Jean-Paul Calderone (exarkun) wrote : Posted in a previous version of this proposal

The change also introduces new `assert` statements. We have since learned that the way these new statements are being used is not helpful. If you want an exception to be raised, raise an exception (and document it and test it).

Revision history for this message
Laurens Van Houtven (lvh) wrote : Posted in a previous version of this proposal

I'm not sure it really introduces any *new* ones: it just keeps the old one. (I only count three assert statements in the diff: one is just diff context; the other two are in the old and new versions of the "uncache" method -- that's the old one being kept.)

That doesn't mean changing the assertion to something else isn't a good idea, of course. That said, this is probably the minimal change that closes this ticket. Would you be okay with the assertions being changed in a different ticket? If you'd like, I will file a bug for and fix it, assuming that gets this ticket moving along :)

Revision history for this message
Tristan Seligmann (mithrandi) wrote :

I still need to rebase the actual branch on trunk, of course. *facepalm* I'll try to look at this soon.

Revision history for this message
Laurens Van Houtven (lvh) wrote :

My only remaining point is that I'm not sure about is the behavior of get. I understand this class isn't a dict, but get() mirrored dict behavior by returning a value or None. Returning a value or raising an exception is the behavior of __getitem__ :)

lp://qastaging/~divmod-dev/divmod.org/829879-pypy-finalization-semantics updated
2703. By Jonathan Jacobs

Accept varargs in Divmod.UnitTest.TestCase.assertThrows.

Author: jjacobs
Reviewer: exarkun

Pass varargs to assertThrows on to the callable, matching the way Python's assertRaises works.

Revision history for this message
Tristan Seligmann (mithrandi) wrote :

The API of get() is not changed in this branch, unless I missed something?

lp://qastaging/~divmod-dev/divmod.org/829879-pypy-finalization-semantics updated
2704. By Tristan Seligmann

Tolerate less strict finalization semantics.

2705. By Tristan Seligmann

Replace assert with raise, and repeat the "dead weakref" check logic.

2706. By Tristan Seligmann

Docstrings, comments, and better argument names.

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: