Opened 11 years ago

Closed 11 years ago

#7315 closed defect (fixed)

Can only forget one GenericDeclaration at a time for some reason

Reported by: kcrisman Owned by: burcin
Priority: major Milestone: sage-4.2.1
Component: calculus Keywords:
Cc: Merged in: sage-4.2.1.alpha0
Authors: Karl-Dieter Crisman Reviewers: Jason Grout
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by kcrisman)

From this thread:

sage: var('m')
sage: var('n')
sage: assume(n, 'integer'); assume(m, 'integer')
sage: sin(n*pi).simplify()
sage: sin(m*pi).simplify()
sage: forget()
sage: sin(m*pi).simplify()
sage: sin(n*pi).simplify()

The problem seems to lie in the last few lines of _forget_all in, where for some reason the loop isn't doing what it should.

Attachments (1)

trac_7315-forget-all.patch (2.0 KB) - added by kcrisman 11 years ago.
Based on 4.2

Download all attachments as: .zip

Change History (7)

comment:1 Changed 11 years ago by kcrisman

Unfortunately, #1163 doesn't fix this either.

comment:2 Changed 11 years ago by kcrisman

  • Description modified (diff)

The problem is actually because list removal is called in GenericDeclaration?.forget(), so this is about behavior of list iteration in Python when you remove elements:

>>> L=[1,2,3,4]
>>> for x in L:
...     L.remove(x)
...     x
...     L
[2, 3, 4]
[2, 4]

So this piece of code is apparently using the wrong/un-Pythonic way of removing items from a list.

Changed 11 years ago by kcrisman

Based on 4.2

comment:3 Changed 11 years ago by kcrisman

  • Authors set to Karl-Dieter Crisman
  • Status changed from new to needs_review

This patch should fix the issue. In fact, it's only because #7084 finally allows more than one such declaration that this bug ever showed up!

comment:4 Changed 11 years ago by jason

  • Status changed from needs_review to positive_review

Great catch. Doctests and documentation passes. Good work.

comment:5 Changed 11 years ago by jason

  • Reviewers set to Jason Grout

comment:6 Changed 11 years ago by mhansen

  • Merged in set to sage-4.2.1.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.