r9y9 / nnmnkwii

Library to build speech synthesis systems designed for easy and fast prototyping.
https://r9y9.github.io/nnmnkwii/latest/
Other
393 stars 73 forks source link

Fix trim_zeros_frames to trim zeros from the end #87

Closed yamachu closed 5 years ago

yamachu commented 5 years ago

Docs of trim_zeros_frames says Remove trailing zeros frames. But it behaves removing both front and back zero frames because of lack of np.trim_zeros 's trim option.

expected

arr = np.array(((0, 0), (1, 1), (2, 2), (0, 0)))
actual = trim_zeros_frames(arr)
# np.array(((0, 0), (1, 1), (2, 2)))

actual

arr = np.array(((0, 0), (1, 1), (2, 2), (0, 0)))
actual = trim_zeros_frames(arr)
# np.array(((0, 0), (1, 1)))

So I thought that it's better to set option trim='b'.

codecov-io commented 5 years ago

Codecov Report

Merging #87 into master will increase coverage by 0.13%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #87      +/-   ##
==========================================
+ Coverage   83.85%   83.98%   +0.13%     
==========================================
  Files          31       31              
  Lines        1697     1711      +14     
==========================================
+ Hits         1423     1437      +14     
  Misses        274      274
Impacted Files Coverage Δ
nnmnkwii/preprocessing/generic.py 99.47% <100%> (+0.04%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1649b3c...fbf98a8. Read the comment docs.

yamachu commented 5 years ago

Thank you for your review.

https://github.com/r9y9/nnmnkwii/pull/87/commits/78743931b18eb24f1f843a1aaa2f9bc2e844eca3 This commit fixed commit that follow your review.

But still trimming issue remains... What do you think about my following suggestion.

These behaviors can be confirmed from these commits. https://github.com/r9y9/nnmnkwii/pull/87/commits/80a4a36026247b86b39df2125f69b8bf6b6d4a2a https://github.com/r9y9/nnmnkwii/pull/87/commits/493487b6d711905b6bec042e8f8259fcd426b7e0

Thanks.

r9y9 commented 5 years ago

Could you put a change log in https://github.com/r9y9/nnmnkwii/blob/1649b3caba671ad203c4d599b5b8b95c168e03ac/docs/changelog.rst#v0019-2019-xx-xx? Thanks!