Ticket #5092 (closed defect: fixed)
Primes()?? gets hung in len call; also bring coverage to 100% for primes.py
| Reported by: | was | Owned by: | was |
|---|---|---|---|
| Priority: | major | Milestone: | sage-4.2 |
| Component: | number theory | Keywords: | |
| Cc: | Work issues: | ||
| Report Upstream: | Reviewers: | Mike Hansen, Karl-Dieter Crisman | |
| Authors: | William Stein | Merged in: | sage-4.2.alpha1 |
| Dependencies: | Stopgaps: |
Description
Attachments
Change History
comment:1 Changed 4 years ago by was
- Summary changed from Primes()?? gets hung in len call to [with patch; needs review] Primes()?? gets hung in len call
comment:2 Changed 4 years ago by was
- Summary changed from [with patch; needs review] Primes()?? gets hung in len call to [with patch; needs review] Primes()?? gets hung in len call; also bring coverage to 100% for primes.py
comment:3 Changed 4 years ago by saliola
- Summary changed from [with patch; needs review] Primes()?? gets hung in len call; also bring coverage to 100% for primes.py to [with patch; needs work] Primes()?? gets hung in len call; also bring coverage to 100% for primes.py
You don't need the first two lines below in cmp anymore.
if isinstance(right, Primes_class):
return 0
return cmp(type(self), type(right))
Otherwise, it's a positive review.
comment:4 Changed 4 years ago by saliola
- Summary changed from [with patch; needs work] Primes()?? gets hung in len call; also bring coverage to 100% for primes.py to [with patch; positive review] Primes()?? gets hung in len call; also bring coverage to 100% for primes.py
comment:5 follow-up: ↓ 6 Changed 4 years ago by mabshoff
- Summary changed from [with patch; positive review] Primes()?? gets hung in len call; also bring coverage to 100% for primes.py to [with patch; needs work] Primes()?? gets hung in len call; also bring coverage to 100% for primes.py
This patch causes one doctest failure:
sage -t -long "devel/sage/sage/sets/set.py"
**********************************************************************
File "/scratch/mabshoff/sage-3.3.alpha4/devel/sage/sage/sets/set.py", line 278:
sage: Primes() < Set(QQ)
Expected:
True
Got:
False
**********************************************************************
Cheers,
Michael
comment:6 in reply to: ↑ 5 Changed 4 years ago by saliola
Replying to mabshoff:
This patch causes one doctest failure:
This is a weird test. I'm not even sure that it says anything meaningful. In fact, according to the documentation of cmp for Set, it doesn't:
\note{If $X < Y$ is true this does \emph{not} necessarily mean
that $X$ is a subset of $Y$. Also, any two sets can be
compared, which is a general Python philosophy.}
comment:7 Changed 4 years ago by AlexGhitza
See #5933, which brings the coverage to 100% and was merged in 3.4.2.rc0.
comment:8 Changed 4 years ago by mabshoff
Well, there is still a potential bug fix in here, so can someone take a look and extra a potential fix?
Cheers,
Michael
comment:9 Changed 4 years ago by mhansen
- Status changed from needs_work to needs_review
- Authors set to William Stein
I've rebased the patch of #5933.
comment:10 Changed 4 years ago by mhansen
- Summary changed from [with patch; needs work] Primes()?? gets hung in len call; also bring coverage to 100% for primes.py to [with patch; needs review] Primes()?? gets hung in len call; also bring coverage to 100% for primes.py
err, on top of #5933
comment:11 Changed 4 years ago by kcrisman
- Status changed from needs_review to positive_review
- Reviewers set to Mike Hansen, Karl-Dieter Crisman
Positive review. Unless you can provide an easily accessible example of where Primes(False) isn't Primes(True), but perhaps the first place that happens is waaay down the road...
comment:12 Changed 4 years ago by mhansen
- Status changed from positive_review to closed
- Resolution set to fixed
- Merged in set to sage-4.2.alpha1
comment:13 Changed 4 years ago by mhansen
- Summary changed from [with patch; needs review] Primes()?? gets hung in len call; also bring coverage to 100% for primes.py to Primes()?? gets hung in len call; also bring coverage to 100% for primes.py

