music-suite / music-pitch

This repo has been merged into: https://github.com/music-suite/music-suite
http://music-suite.github.io
BSD 3-Clause "New" or "Revised" License
9 stars 9 forks source link

isLeap m3 = False; Music-pitch 1.8 #44

Closed meditans closed 9 years ago

meditans commented 9 years ago

It should be true. The haddock documentation reports:

Returns whether the given interval is a leap (larger than a second). Only the diatonic number is taken into account, so _A2 is considered a step and m3 a leap, even though they have the same number of semitones.

Also, isStep m3 = True

Steps to reproduce the problem: $ ghci

import Music.Prelude isLeap m3 False

meditans commented 9 years ago

That is because you defined isLeap in terms of the diatonic steps, with:

isLeap (Interval (a, d)) = (abs d) > 2

But as

m3 ^. (from interval') = (ChromaticSteps = -1, DiatonicSteps = 2)

The correct definition would be, I think:

isLeap (Interval (a, d)) = (abs d) > 1

We should also write some test to avoid similar errors. I could do a PR in a few days, but in the meantime you can correct it, if you want.