Opened 4 years ago
Last modified 11 months ago
#19154 needs_work defect
duplicate method in finite_words: lps_lengths, lengths_lps
Reported by:  sschanck  Owned by:  

Priority:  major  Milestone:  sageduplicate/invalid/wontfix 
Component:  combinatorics  Keywords:  palindromes, sd69 
Cc:  nadialafreniere, mlapointe, tmonteil, slabbe  Merged in:  
Authors:  Stépanie Schanck  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  u/sschanck/19154 (Commits)  Commit:  502a70ff9783ac2cff7349a58d8042ea8507e989 
Dependencies:  Stopgaps: 
Description (last modified by )
In finite_word, there are two functions that do the same thing : lps_lengths and lengths_lps. The second one is quadratic and the first one is linear. In order not to delete the longest function, we create a new one calling the fastest by default and the longest if the naive_algorithm parameter was set to True.
Change History (11)
comment:1 Changed 4 years ago by
 Cc nadialafreniere mlapointe added
 Component changed from PLEASE CHANGE to combinatorics
 Description modified (diff)
 Keywords palindromes sd69 added
 Type changed from PLEASE CHANGE to defect
comment:2 Changed 4 years ago by
 Branch set to u/sschanck/19154
comment:3 Changed 4 years ago by
 Commit set to 70f7b1c5d206e8627f3d124a28b7083e3a82313a
 Description modified (diff)
 Status changed from new to needs_review
 Summary changed from Deleting a duplication of a function to Managing a duplication of a function
comment:4 Changed 4 years ago by
 Commit changed from 70f7b1c5d206e8627f3d124a28b7083e3a82313a to efe27716f39d547d18d95feb246d6aae0162eeb1
comment:5 Changed 4 years ago by
 Commit changed from efe27716f39d547d18d95feb246d6aae0162eeb1 to 1cb84a0de023ebe6c2a23d3de24b958641514559
Branch pushed to git repo; I updated commit sha1. New commits:
1cb84a0  no need to write if naive_algorithm == True, just if naive_algorithm

comment:6 Changed 4 years ago by
 Commit changed from 1cb84a0de023ebe6c2a23d3de24b958641514559 to 502a70ff9783ac2cff7349a58d8042ea8507e989
Branch pushed to git repo; I updated commit sha1. New commits:
502a70f  Fixing indexes in palindromes

comment:8 Changed 2 years ago by
 Cc tmonteil added
comment:9 followup: ↓ 10 Changed 2 years ago by
 Milestone changed from sage6.9 to sageduplicate/invalid/wontfix
 Status changed from needs_work to needs_review
I think this ticket should be closed. It would need to be rewritten to agree with the actual version of Sage and I don't think Stéphanie (or someone else) would do it. Moreover, I'm not convinced that creating a new function is the right thing to do to solve that problem.
comment:10 in reply to: ↑ 9 Changed 11 months ago by
 Cc slabbe added
 Status changed from needs_review to needs_work
Replying to nadialafreniere:
I think this ticket should be closed. It would need to be rewritten [...]. Moreover, I'm not convinced that creating a new function is the right thing to do to solve that problem.
I agree, but the issue still exists, so I'm setting the ticket to needs_work instead.
comment:11 Changed 11 months ago by
 Summary changed from Managing a duplication of a function to duplicate method in finite_words: lps_lengths, lengths_lps
Branch pushed to git repo; I updated commit sha1. New commits:
Renamed lps_lengths to create another one that can manage the duplicate.