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

Should QQ[x, y, ...] syntax be removed? #14414

Open
skirpichev opened this Issue Mar 7, 2018 · 6 comments

Comments

Projects
None yet
2 participants
@skirpichev
Contributor

skirpichev commented Mar 7, 2018

That's a syntactic sugar for QQ.poly_ring(x, y, ...). Clearly, it's coming from standard mathematical notation, but I doubt that this syntax is pythonic.

For more fun, __call__ overridden to create elements of the domain, not construct field of fractions (with .frac_field()), as you might guess from the above example with rings.

skirpichev added a commit to skirpichev/diofant that referenced this issue Mar 7, 2018

@asmeurer

This comment has been minimized.

Member

asmeurer commented Mar 7, 2018

I don't think it should be removed.

@skirpichev

This comment has been minimized.

Contributor

skirpichev commented Mar 7, 2018

Could you explain why do you think so? Should then __call__ be overriden for frac_field?

@skirpichev

This comment has been minimized.

Contributor

skirpichev commented Mar 7, 2018

btw, issue is not new

@skirpichev

This comment has been minimized.

Contributor

skirpichev commented Mar 7, 2018

(If it's not clear, I'm not suggesting to change string parsing for composite domains.)

@asmeurer

This comment has been minimized.

Member

asmeurer commented Mar 7, 2018

It's a convenient syntax. If __getitem__ made sense on a ring object in the usual sense (indexing), then it would indeed be bad, because people would expect it to index. But as there is no meaningful way to index a ring, even a finite ring, I don't see any harm in the syntactic sugar. Also it's been there for some time, so my default position for any breaking change is to maintain the existing API.

For __call__, again, the API already exists, so it doesn't matter. I actually have been caught off guard several times that __call__ doesn't create a fraction field, so actually if I were designing this from scratch I would make it do that. But it's too late now either way.

@skirpichev

This comment has been minimized.

Contributor

skirpichev commented Mar 7, 2018

If getitem made sense on a ring object in the usual sense (indexing), then it would indeed be bad

Well, __getitem__ was defined not on a ring object, but for the Domain class. I'm not sure if this class assume some algebraic structure at all. It's like a set and there are __contains__ method implemented which tests that element belongs to this domain (e.g. QQ(2) in ZZ, but QQ(1, 2) in ZZ is False).

Does it make sense to index such sets? Sure, sometimes. For example, ZZ is countable - so we could implement __iter__ or even assign natural indexes. Same for QQ or any algebraic number field.

I don't see any harm in the syntactic sugar.

It might look familiar to mathematicians, but totally misleading from the language point of view. No reason to offer a sequence interface if it doesn't make sense for the object (or, worse, if it does make sense, but totally different).

For call, again, the API already exists, so it doesn't matter.

I believe, it should matter if existing API is inconsistent and/or misleading.

Anyway, thank you for explanation. Fell free to close issue if it does make sense for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment