Opened 12 years ago

Closed 12 years ago

#8682 closed defect (fixed)

Improve AlgebraicScheme_subscheme.__init__ and AmbientSpace._validate

Reported by: novoselt Owned by: AlexGhitza
Priority: major Milestone: sage-4.4.4
Component: algebraic geometry Keywords:
Cc: Merged in: sage-4.4.4.alpha0
Authors: Andrey Novoseltsev Reviewers: Alex Ghitza
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

Creation of a subscheme given by polynomial equations in some ambient space involves converting the input to polynomials in the correct ring and checking that these polynomials are "OK", e.g. that they are homogeneous for the projective space. There are the following (little) problems with the current realization:

  • converting to the coordinate ring is done in _validate method of ambient spaces, but it is the same for all of them and in general I would expect that a method with such a name just checks something without modifying the input
  • if a subscheme is constructed using an ideal of a wrong ring, but polynomials can be converted into the coordinate ring of the ambient space, then wrong ideal will be saved for later use
  • _validate is not listed as a mandatory method for overriding by subclasses of AmbientSpace

The attached patch makes the following:

  • all conversions are done in __init__ of the subscheme
  • _validate of AmbientSpace's must check that the polynomials are OK, but they are already guaranteed to lie in the correct ring
  • _validate is listed as a method which must be overridden
  • error messages in exceptions include only the polynomial that lead to the error, not the whole input

Apply on top of #8675.

Attachments (1)

trac_8682_improve_algebraic_subscheme_init.patch (9.8 KB) - added by novoselt 12 years ago.

Download all attachments as: .zip

Change History (4)

comment:1 Changed 12 years ago by novoselt

  • Status changed from new to needs_review

comment:2 Changed 12 years ago by AlexGhitza

  • Reviewers set to Alex Ghitza
  • Status changed from needs_review to positive_review

Looks good to me.

comment:3 Changed 12 years ago by mhansen

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