Opened 10 years ago

Closed 10 years ago

#11627 closed enhancement (fixed)

Turn Fan(discard_warning) into an error

Reported by: vbraun Owned by: mhampton
Priority: major Milestone: sage-4.7.2
Component: geometry Keywords:
Cc: novoselt Merged in: sage-4.7.2.alpha3
Authors: Andrey Novoseltsev Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by vbraun)

There is no easy way to re-enable the warning after it has been shown once. A typical use case is that you have a mistake in your fan data and you want to fix it. The fact that the warning for duplicate cones is only shown once in the Sage session is very annoying.

Apply trac_11627_alternative.patch

Attachments (2)

trac_11627_make_fan_raise_errors.patch (4.8 KB) - added by vbraun 10 years ago.
Initial patch
trac_11627_alternative.patch (10.8 KB) - added by novoselt 10 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 10 years ago by novoselt

  • Component changed from algebraic geometry to geometry
  • Owner changed from AlexGhitza to mhampton

OK, I'll change it ;-)

comment:2 Changed 10 years ago by vbraun

I do have a patch, I just haven't had the time to finish it.

comment:3 Changed 10 years ago by novoselt

Namewise, how about replacing disacard_warning, which will not make sense anymore, with discard_faces=False that can be set to True to allow it?

Changed 10 years ago by vbraun

Initial patch

comment:4 Changed 10 years ago by vbraun

Here is my take on it. I renamed it to check_duplicate_cones=<bool> to make clear that it is related to the check optional argument.

comment:5 Changed 10 years ago by vbraun

  • Authors set to Volker Braun
  • Status changed from new to needs_review

Any thoughts on the patch?

comment:6 Changed 10 years ago by novoselt

Hi Volker, sorry for the delay - travel/jetlag/work ;-)

Discarding faces and repeated cones is quite natural when they come from some construction and I have already wished a few times that it would be possible to drop them without using check=True option which does much more work. In fact, that's the reason why there is a function discard_faces. So I propose this patch which adds a discard_faces=False keyword which works independently of check. I have also fixed the code and documentation pieces that were relying on the old behaviour, I don't get now any errors for tests in geometry or schemes directories.

Changed 10 years ago by novoselt

comment:7 Changed 10 years ago by vbraun

  • Authors changed from Volker Braun to Andrey Novoseltsev
  • Description modified (diff)
  • Reviewers set to Volker Braun
  • Status changed from needs_review to positive_review

Looks good!

comment:8 Changed 10 years ago by leif

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