Review: Needs Fixing
- line 844 - 849: you should pass can_shortcuts=True as well
- Constants should be CAPS_SEPARATED_BY_UNDERSCORES
700 +min_fragment_size = 5
701 +min_block_size = 70
702 +max_typo_size = 3
- Line 813: You can put the import on the same line.
- Wouldn't it be better to rework this part, the _length_of_equal_blocks() and
_length_of_longest_equal_block() functions?
724 + if _length_of_equal_blocks(diff_no_typos) >= min_block_size or \
725 + _length_of_longest_equal_block(diff_no_typos) > len(small) *
2 / 3:
726 + return True
727 + else:
728 + return False
Better:
return my_function_with_both_checks(diff_no_typos, small)
and:
def my_function_with_both_checks(diff, smal):
# Check bla bla bla
length = 0
for element in diff:
if element[0] == "equal":
new_length = _op_length(element)
if new_length >= min_block_size:
length += new_length
if length >= min_block_size:
return True
# Now check bla bla bla.
length = 0
....
....
....
if length > len(small) * 2 / 3:
return True
return False
(Maybe you even do not need a new method for this and can just put this in
songs_probably_equal())
- You have a conflict (merge trunk and resolve it)
--
https://code.launchpad.net/~patrick-zakweb/openlp/duplicate-removal-review/+merge/149724
Your team OpenLP Core is subscribed to branch lp:openlp.
_______________________________________________
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openlp-core
More help : https://help.launchpad.net/ListHelp
