Opened 13 years ago

Closed 13 years ago

#5482 closed defect (fixed)

Quotient ring can be created without generator names

Reported by: justin Owned by: justin
Priority: major Milestone: sage-4.3
Component: algebra Keywords:
Cc: Merged in: sage-4.3.alpha0
Authors: Alex Ghitza Reviewers: Mike Hansen
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

The following code works:

sage: R.<x> = PolynomialRing(QQ)
sage: f = x^2-1
sage: S = R.quotient_by_principal_ideal(f)

but then this fails:

sage: S
 ---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
...[snip]
/Users/tmp/sage-3.4.alpha0/local/lib/python2.5/site-packages/sage/structure/category_object.so in sage.structure.category_object.CategoryObject.variable_names (sage/structure/category_object.c:3530)()

ValueError: variable names have not yet been set using self._assign_names(...)

The routine should require that the name(s) be provided.

Attachments (2)

sage-5482.patch (1.6 KB) - added by justin 13 years ago.
trac_5482.patch (1.5 KB) - added by AlexGhitza 13 years ago.
apply this patch only

Download all attachments as: .zip

Change History (11)

Changed 13 years ago by justin

comment:1 Changed 13 years ago by justin

  • Summary changed from Quotient ring can be created without generator names to [With patch; needs review] Quotient ring can be created without generator names

The fix is to require the generator name at creation time, not when the ring is used.

comment:2 Changed 13 years ago by cremona

Why do you change the parameter name from "names" to "name"? Is this function only used for univariate polynomial rings? If so, fine.

comment:3 Changed 13 years ago by mabshoff

  • Milestone set to sage-3.4.2

comment:4 follow-up: Changed 13 years ago by was

  • Summary changed from [With patch; needs review] Quotient ring can be created without generator names to [With patch; needs work] Quotient ring can be created without generator names

REFEREE REPORT:

  1. It *must* be "names" not "name", so the R.<x> = foo notation works.
  1. Every patch has to have a doctest that illustrates that it fixes the bug.

comment:5 in reply to: ↑ 4 Changed 13 years ago by justin

  • Owner changed from was to justin
  • Status changed from new to assigned

Replying to was:

REFEREE REPORT:

  1. It *must* be "names" not "name", so the R.<x> = foo notation works.

I discovered that while adding doctests. I'll reverse that change.

  1. Every patch has to have a doctest that illustrates that it fixes the bug.

Doctests?

comment:6 Changed 13 years ago by AlexGhitza

  • Component changed from algebraic geometry to algebra
  • Summary changed from [With patch; needs work] Quotient ring can be created without generator names to Quotient ring can be created without generator names

comment:7 Changed 13 years ago by AlexGhitza

  • Authors set to Alex Ghitza
  • Status changed from needs_work to needs_review

I attached a new patch that assigns names automatically if they are not specified by the user, e.g. a quotient of R.<x> will have variable name xbar. This is standard behaviour in other places in Sage.

Apply trac_5482.patch only.

Changed 13 years ago by AlexGhitza

apply this patch only

comment:8 Changed 13 years ago by mhansen

  • Reviewers set to Mike Hansen
  • Status changed from needs_review to positive_review

Looks good to me.

comment:9 Changed 13 years ago by mhansen

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