Opened 14 years ago
Closed 13 years ago
#3571 closed defect (fixed)
[with patch, positive review] ivalue field in integer_mod.pyx shouldn't be public
Reported by: | craigcitro | Owned by: | craigcitro |
---|---|---|---|
Priority: | major | Milestone: | sage-3.3 |
Component: | basic arithmetic | Keywords: | |
Cc: | Merged in: | ||
Authors: | Reviewers: | ||
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
The ivalue
field for IntegerMod_int
is public
, but it shouldn't be. The following is very frightening, for instance:
sage: R = Integers(10) sage: x = R(2) sage: x 2 sage: x.ivalue = 33 sage: x 33 sage: R(2) 33
It's easy to make this field no longer be public, but lots of things are using the fact that it is, so one needs to go through and make everything work correctly again.
Attachments (1)
Change History (4)
comment:1 Changed 13 years ago by
- Status changed from new to assigned
- Summary changed from ivalue field in integer_mod.pyx shouldn't be public to [with patch, needs review] ivalue field in integer_mod.pyx shouldn't be public
Changed 13 years ago by
comment:2 Changed 13 years ago by
- Summary changed from [with patch, needs review] ivalue field in integer_mod.pyx shouldn't be public to [with patch, positive review] ivalue field in integer_mod.pyx shouldn't be public
comment:3 Changed 13 years ago by
- Milestone changed from sage-3.4.1 to sage-3.3
- Resolution set to fixed
- Status changed from assigned to closed
Merged in Sage 3.3.alpha1
Cheers,
Michael
Note: See
TracTickets for help on using
tickets.
The attached patch fixes this issue, and in fact, gives about a 1.5-2X speedup on multiplying
IntegerMod_int
s. The interesting part is that previous to this patch, theivalue
field was occasionally being accessed as a Python attribute instead of a Cython attribute! That's why it broke if we made the field notpublic
in the first place. Oops.BEFORE:
AFTER: