Opened 2 years ago
Closed 2 years ago
#30754 closed enhancement (fixed)
refresh Cfinite sequences
Reported by:  chapoton  Owned by:  

Priority:  major  Milestone:  sage9.3 
Component:  commutative algebra  Keywords:  
Cc:  slelievre  Merged in:  
Authors:  Frédéric Chapoton  Reviewers:  Samuel Lelièvre 
Report Upstream:  N/A  Work issues:  
Branch:  0da8bca (Commits, GitHub, GitLab)  Commit:  0da8bcad8d684c26a1a7f8a473ff2b0847a7ebc0 
Dependencies:  Stopgaps: 
Description (last modified by )
Full flake8 and some small changes in code.
Change History (8)
comment:1 Changed 2 years ago by
Branch:  → u/chapoton/30754 

Commit:  → 3fdf884dcc51fd0e83f0cf4ca92a6fba867fb53d 
Status:  new → needs_review 
comment:2 Changed 2 years ago by
Commit:  3fdf884dcc51fd0e83f0cf4ca92a6fba867fb53d → 5681d88f341d679c0704e69ce979d9370beecf59 

Branch pushed to git repo; I updated commit sha1. New commits:
5681d88  fix pyflakes in cfinite

comment:3 Changed 2 years ago by
Positive review if bots are happy.
Or if you want to go further, but no pressure to do that here, some ideas.
One could check whether
 some
== 0
could be.is_zero()
 some
== 1
could be.is_one()
 some
X(0)
could beX.zero()
 some
X(1)
could beX.one()
 some
X(n / d)
could ben // d
One could consider minor rewriting in some places.
for k, a in enumerate(part.numerator()):  a = QQbar(a) + a = QQbar(a) if k % 2 else QQbar(a) bino = binomial(n + m  k, m)  c += bino * SR(((1)**k * a * b**(k  m  1)).radical_expression()) + c += bino * SR((a * b**(k  m  1)).radical_expression())
 if i != 0: + if i:
 co = coefficients[::1]  co.extend([0] * (len(values)  deg))  x = self.polynomial_ring().gen()  den = 1 + sum(x**(n + 1) * co[n] for n in range(deg))  num = values[0] + sum(x**n * (values[n] +  sum(values[k] * co[n  1  k]  for k in range(n)))  for n in range(1, len(values))) + co = coefficients[::1] + [0] * (len(values)  deg) + R = self.polynomial_ring() + den = R([1] + co[:deg]) + v = lambda m: sum((values[k] * co[m  k] for k in range(m + 1)), values[m + 1]) + num = R([v(n) for n in range(1, len(values)  1)])
 sequence = sequence[:1] + del sequence[1]
 l = len(sequence)  while l and sequence[l  1] == 0:  l = 1  sequence = sequence[:l] + while sequence and sequence[1] == 0: + del sequence[1] + l = len(sequence)
 offset = next((i for i, x in enumerate(sequence) if x != 0), None) + offset = next((i for i, x in enumerate(sequence) if x), None)
Final suggestion, even more out of scope here.
Is from_recurrence
slightly off when values
is longer than coefficients
?
For example, should 6
repeat here?
sage: a = C.from_recurrence([1, 1], [0, 1, 1, 5]) sage: a[:9] [0, 1, 1, 5, 6, 6, 11, 17, 28]
comment:4 Changed 2 years ago by
Commit:  5681d88f341d679c0704e69ce979d9370beecf59 → 0da8bcad8d684c26a1a7f8a473ff2b0847a7ebc0 

Branch pushed to git repo; I updated commit sha1. New commits:
0da8bca  more details in cfinite sequences

comment:5 Changed 2 years ago by
This is a bug in __getitem__
, already there before the changes made here. I have no idea how to fix that.
sage: C = CFiniteSequences(QQ) sage: a = C.from_recurrence([1, 1], [0, 1, 1, 5]) sage: a[:9] [0, 1, 1, 5, 6, 6, 11, 17, 28]
comment:6 Changed 2 years ago by
Cc:  slelievre added 

Description:  modified (diff) 
Reviewers:  → Samuel Lelièvre 
I know the getitem bug is not from the changes here, no worries.
Thanks for the extra details you fixed following suggestions in comment:3.
I get how the lambda function suggestion was not so appealing.
Another way to streamline, hopefully more convincing:
 num = R([values[0]] +  [values[n] + sum(values[k] * co[n  1  k]  for k in range(n))  for n in range(1, len(values))]) + num = R([v + sum(values[k] * co[n  1  k] for k in range(n)) + for n, v in enumerate(values)])
since when n
is zero the sum is empty, so no need to specialcase it.
But up to you and again, no pressure.
Positive review on my behalf if bots are happy, with or without this change.
comment:8 Changed 2 years ago by
Branch:  u/chapoton/30754 → 0da8bcad8d684c26a1a7f8a473ff2b0847a7ebc0 

Resolution:  → fixed 
Status:  positive_review → closed 
New commits:
refresh Cfinite sequences (flake8 and code details)