Opened 7 years ago

Closed 6 years ago

#14548 closed defect (wontfix)

coefficients method for polynomials and power series

Reported by: roed Owned by: AlexGhitza
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: algebra Keywords:
Cc: niles Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: u/chapoton/14548 (Commits) Commit: 7b56441867da56bb3d41ef1829432bf098f3489d
Dependencies: Stopgaps:

Description (last modified by rws)

In #2081 we introduced a coefficients method for univariate polynomials and power series, which only returns the nonzero coefficients.

sage: R.<x> = QQ[]
sage: (x^2+1).coefficients()
[1, 1]
  • The result for x^2 + 1 should be [1, 0, 1].
  • We should add a method coefficients_nonzero for univariate and multivariable polynomials and power series that does what coefficients now does.
  • We should deprecate the coefficients method on multivariate polynomials.

See also #17518 and the sage-devel discussion linked there.

Change History (15)

comment:1 Changed 7 years ago by vbraun

How about nonzero_coefficients instead of the french way.

Your desired behavior is already implemented in the coeffs() method. So +1 for making coefficients an alias to coeffs.

It would be nice to be able to iterate over (coefficient, exponent) without constructing a sparse dictionary (using the dict() method) first. Call it iteritems()? monomial_iter()?

comment:2 Changed 7 years ago by roed

The reason I proposed the French way is that tab completion will reveal the existence of coefficients_nonzero. Hm.... maybe we could have tab completion be smart about this and c<TAB> would also return methods with exactly one underscore where the second word started with c? ` Anyway, I agree that having an iterator over pairs would be nice. I like iteritems() since I would expect monomial_iter() to yield monomials rather than pairs.

comment:3 Changed 7 years ago by vbraun

I made #14552 to collect some improvements for the tab completion. I don't like ugly method names just because that works nicer in the default tab completer.

comment:4 Changed 7 years ago by niles

  • Cc niles added

See also #9457

p.s. I'm in favor of index-style method names, which I believe is the same as French style. Moreover, I am also in favor of using, for this ticket, whatever method naming style is standard, or most popular, or recommended in current Sage practice. If there is such a style ;)

comment:5 follow-up: Changed 7 years ago by vbraun

Thats PEP8: lower_case_with_underscores, not underscores_with_case_lower

comment:6 in reply to: ↑ 5 Changed 7 years ago by niles

Replying to vbraun:

Thats PEP8: lower_case_with_underscores, not underscores_with_case_lower

Reading here

http://www.python.org/dev/peps/pep-0008/

I do see lower_case_with_underscores as an example of a naming style, but I believe this is meant to indicate that the method names are all lower case and separated by underscores, not what order the words should go in. It's in the "Descriptive" section anyway, not the "Prescriptive" section. I didn't see anything about placement of adjectives; is there a different PEP for that?

comment:7 Changed 7 years ago by vbraun

Its not spelled out, probably because the PEP authors didn't even think about word orders that are not correct English. But if you read the PEP then its clear that this is an underlying assumption.

comment:8 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:9 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:10 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:11 Changed 6 years ago by chapoton

  • Branch set to u/chapoton/14548
  • Commit set to 7b56441867da56bb3d41ef1829432bf098f3489d

Here a git branch, touching only polynomials and not power series, and also missing the appropriate deprecations.


New commits:

7b56441trac #14548 first step

comment:12 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:13 Changed 6 years ago by rws

  • Description modified (diff)
  • Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix
  • Status changed from new to needs_review

See also #17518 and the sage-devel discussion linked there. #17518 clashes with this ticket because it implements a different solution, i.e., a sparse keyword. This is certainly better than changing a much used function name. Note also that with #17518 and #17438 the interface to the same functions in SR and PowerSeriesRing is unified.

I therefore propose to obsolete this ticket.

comment:14 Changed 6 years ago by mmezzarobba

  • Status changed from needs_review to positive_review

comment:15 Changed 6 years ago by vbraun

  • Resolution set to wontfix
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.