Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

can't make vector of ints #3847

Closed
saliola opened this issue Aug 14, 2008 · 5 comments
Closed

can't make vector of ints #3847

saliola opened this issue Aug 14, 2008 · 5 comments

Comments

@saliola
Copy link

saliola commented Aug 14, 2008

sage: vector([int(0)])
Traceback (most recent call last):
...
TypeError: unable to find a common ring for all elements

Shouldn't ints be coerced to Integers?

Component: algebra

Issue created by migration from https://trac.sagemath.org/ticket/3847

@saliola saliola added this to the sage-3.1.3 milestone Aug 14, 2008
@aghitza
Copy link

aghitza commented Sep 8, 2008

comment:1

Attachment: 3847-vector.patch.gz

This issue has also come up in other guises on sage-devel or sage-support (can't seem to be able to find the threads right now, though).

It all boils down to Sequence(), which takes a list of things and returns a sequence of things that lie in the same "universe", if canonical coercions are possible. So if you give it a list containing an RDF, an Integer, and a Rational, it will give you back a sequence of three RDF's. The problem is with the builtin Python types (int, long, float, complex), for which we of course have no coercions. So given a bunch of ints, Sequence tries to see where in the Sage hierarchy they fit, finds nothing, and sends them back without a common "universe". This makes vector() throw an exception, because a vector should be over a ring.

The patch fixes this by giving Sequence() an optional parameter use_sage_types, defaulting to False. If the parameter is True, Sequence() catches the builtin Python types and makes them into the appropriate Sage objects, then carries on. I am making vector() always pass use_sage_types=True to Sequence(). This fixes the unpleasant behavior reported in this ticket, and gives vector() a bit more flexibility -- see the new doctests for more examples.

Sequence() is a fairly fundamental class and I didn't want to change its default behavior for fear of speed degradation in places other than vector().

@JohnCremona
Copy link
Member

comment:2

Review: I think this has been done very well. The default behaviour for Sequence() has not changed, so there should be no effect anywhere other than for the Vector constructor, which does what it should.

The patch applies cleanly to 3.1.2.rc0 and all doctests in sage.structure and sage.modules pass.

There may be other places where Sequence() is used where this functionality would be useful.

@JohnCremona JohnCremona changed the title can't make vector of ints [with positive review] can't make vector of ints Sep 8, 2008
@sagetrac-mabshoff

This comment has been minimized.

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Sep 14, 2008

comment:3

Oops, this patch nearly escaped. Note that the two spaces between positive and review let this review escape from the report.

Cheers,

Michal

@sagetrac-mabshoff sagetrac-mabshoff mannequin changed the title [with positive review] can't make vector of ints can't make vector of ints Sep 14, 2008
@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Sep 15, 2008

comment:4

Merged in Sage 3.1.2.rc4

@sagetrac-mabshoff sagetrac-mabshoff mannequin modified the milestones: sage-3.1.3, sage-3.1.2 Sep 15, 2008
@sagetrac-mabshoff sagetrac-mabshoff mannequin closed this as completed Sep 15, 2008
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants