#30275 closed defect (fixed)
Replace AssertionError with ValueError
Reported by:  Michael Jung  Owned by:  

Priority:  major  Milestone:  sage9.2 
Component:  manifolds  Keywords:  
Cc:  Eric Gourgoulhon, Travis Scrimshaw, Matthias Köppe  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 2 years ago by
Milestone:  sage9.2 → sage9.3 

comment:3 Changed 2 years ago by
Branch:  → u/ghmjungmath/replace_assertionerror_with_valueerror 

comment:4 Changed 2 years ago by
Authors:  → Michael Jung 

Commit:  → 787bc4b5db9e06093df9a6b5fa2722de69427245 
Status:  new → needs_review 
comment:5 Changed 2 years ago by
Dependencies:  #30181, #30261, #30239 

comment:6 Changed 2 years ago by
Description:  modified (diff) 

comment:9 Changed 2 years ago by
Commit:  787bc4b5db9e06093df9a6b5fa2722de69427245 → a20a6d29d125aae956111529df663be4b14943c7 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
a20a6d2  Trac #30275: AssertionError replaced

comment:11 Changed 2 years ago by
Reviewers:  → Travis Scrimshaw 

Status:  needs_review → positive_review 
Thanks. LGTM.
comment:12 followup: 13 Changed 2 years 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 Changed 2 years ago by
Replying to ghmjungmath:
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 2 years ago by
Branch:  u/ghmjungmath/replace_assertionerror_with_valueerror → a20a6d29d125aae956111529df663be4b14943c7 

Resolution:  → fixed 
Status:  positive_review → closed 
comment:15 Changed 2 years ago by
Milestone:  sage9.3 → sage9.2 

New commits:
Trac #30275: AssertionError replaced