#30275 closed defect (fixed)
Replace AssertionError with ValueError
Reported by: | gh-mjungmath | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.2 |
Component: | manifolds | Keywords: | |
Cc: | egourgoulhon, tscrim, mkoeppe | Merged in: | |
Authors: | Michael Jung | Reviewers: | Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | a20a6d2 (Commits, GitHub, GitLab) | Commit: | a20a6d29d125aae956111529df663be4b14943c7 |
Dependencies: | Stopgaps: |
Description (last modified by )
Turns out that an AssertionError
is inapropriate to raise when immutable elements are tried to be changed (see here):
exception AssertionError
Raised when an assert statement fails.
since no assert
statements are involved. A better choice would be a ValueError
.
Change History (15)
comment:1 Changed 2 years ago by
- Description modified (diff)
comment:2 Changed 22 months ago by
- Milestone changed from sage-9.2 to sage-9.3
comment:3 Changed 22 months ago by
- Branch set to u/gh-mjungmath/replace_assertionerror_with_valueerror
comment:4 Changed 22 months ago by
- Commit set to 787bc4b5db9e06093df9a6b5fa2722de69427245
- Status changed from new to needs_review
comment:5 Changed 22 months ago by
- Dependencies #30181, #30261, #30239 deleted
comment:6 Changed 22 months ago by
- Description modified (diff)
comment:7 Changed 22 months ago by
Why the change in algebras/commutative_dga.py
?
comment:8 Changed 22 months ago by
That is not supposed to be there. Change it.
comment:9 Changed 22 months ago by
- Commit changed from 787bc4b5db9e06093df9a6b5fa2722de69427245 to a20a6d29d125aae956111529df663be4b14943c7
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
a20a6d2 | Trac #30275: AssertionError replaced
|
comment:10 Changed 22 months ago by
There we go.
comment:11 Changed 22 months ago by
- Reviewers set to Travis Scrimshaw
- Status changed from needs_review to positive_review
Thanks. LGTM.
comment:12 follow-up: ↓ 13 Changed 22 months ago by
Thank you for the quick review.
What about the pyflakes errors? It has nothing to do with the ticket, but shouldn't that be polished at some point?
comment:13 in reply to: ↑ 12 Changed 22 months ago by
Replying to gh-mjungmath:
Thank you for the quick review.
No problem.
What about the pyflakes errors? It has nothing to do with the ticket, but shouldn't that be polished at some point?
Yes, but that is something for another ticket. You probably can take care of all of those at once for the manifolds folder.
comment:14 Changed 22 months ago by
- Branch changed from u/gh-mjungmath/replace_assertionerror_with_valueerror to a20a6d29d125aae956111529df663be4b14943c7
- Resolution set to fixed
- Status changed from positive_review to closed
comment:15 Changed 22 months ago by
- Milestone changed from sage-9.3 to sage-9.2
New commits:
Trac #30275: AssertionError replaced