Opened 10 years ago
Last modified 5 years ago
#8766 new defect
document the _iadd_ and _imul_ special integer.pyx methods, which mutate self
Reported by: | was | Owned by: | AlexGhitza |
---|---|---|---|
Priority: | major | Milestone: | sage-6.4 |
Component: | basic arithmetic | Keywords: | |
Cc: | Merged in: | ||
Authors: | Reviewers: | ||
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
I looked in integer.pyx at the two methods _iadd_ and _imul_, which both mutate self, e.g., allow for:
sage: a = 2010 sage: a._imul_(19) sage: a 38190
I expected to find a bug exciting docstring about how these methods are unsafe, etc. Instead, there is *NOTHING* -- not even a doctest or docstring at all.
I also wish there were versions of these:
sage: a.unsafe_add_inplace sage: a.unsafe_mul_inplace
that could be used in code, and would make their unsafe nature clear to the reader without having to read any docs.
Change History (7)
comment:1 Changed 10 years ago by
- Description modified (diff)
comment:2 Changed 10 years ago by
comment:3 Changed 10 years ago by
This is odd. From their names one would expect them to be used in imul and iadd somewhere in the hierarchy, just like _repr_ is used in repr, so that they will be used for:
sage: a = 1 sage: a*=5
as documented here: http://docs.python.org/reference/datamodel.html. However, this is not the case. It may be a bug (or yet to be implemented feature).
That is not a bug -- it is done on purpose. The reason is because integers are meant to be *immutable*, since they have a hash method. If _imul_ were used by imul, then people would be mutating integers left and right by accident, and vast amounts of code would consequently have subtle bugs all over the place.
There might have been a time (maybe a few weeks in 2006) when imul did indeed call _imul_ in Sage, so the name might be a historical remnant.
Personally, I think the best thing would be:
(1) Rename _imul_ and _iadd_ to something like _unsafe_inplace_mul_, _unsafe_inplace_add_
(2) Document them.
William
comment:4 Changed 6 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:5 Changed 6 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:6 Changed 6 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:7 Changed 5 years ago by
- Milestone changed from sage-6.3 to sage-6.4