Proposal: Output [Int: Pitch.Spelling] from spell()

[bwetherfield]

This is how code and testing would look if spell() output Pitch.Spelling rather than SpelledPitch values. What do we think, @jsbean?

Could octave information be something we inject?

I think it could be nice if spell() had a generic parameter, that both Pitch.Spelling and SpelledPitch conformed to, and hence could potentially spit out one or the other...?

[bwetherfield]

For now I can try wrapping Pitch.Spelling and SpelledPitch in protocol witnesses!

[jsbean]

@bwetherfield, good question. What is the reason not to return the full SpelledPitch type?

I think what I was thinking for this would be to return all of the data you would need to notationally represent the input Pitch values. Given that we get the octave information on the way in, why not bind it back into the result? I would argue that if we used Pitch.Class as the input element (in this case, [Int: Pitch.Class]), then it would make sense to return Pitch.Spelling.

I see the types kind of like this:

Unspelled	Spelled
Pitch.Class	Pitch.Spelling
Pitch	SpelledPitch

I could definitely hear the argument that this feels clumsy / crooked. At some point SpelledPitchClass existed, but ended up just being a nominal wrapper for Pitch.Spelling.

In this case, I could imagine defining this operation roughly as (where F is the containing "functor" type):

• (F<Pitch.Class>) -> F<Pitch.Spelling>, or
• (F<Pitch>) -> F<SpelledPitch>

Otherwise, the user would have to bind it back themselves. Could you imagine a scenario where a user would not want the full bundle of info?

Are your concerns re: SpelledPitch logical, logistical, or performance-related?

[bwetherfield]

So, in this commit the octave term in expected seems a bit hacky - that's why I thought a pitch class perspective might be helpful especially for testing.

[bwetherfield]

Could Unspelled and Spelled be protocols that their respective columns conform to?

[jsbean]

So, in this commit the octave term in expected seems a bit hacky - that's why I thought a pitch class perspective might be helpful especially for testing.

Yeah. I was thinking this would always be used to spell real-wold Pitch values, not theoretical Pitch.Class values. But now I am starting to see how you are thinking.

Could Unspelled and Spelled be protocols that their respective columns conform to?

So you are thinking that we should really define the PitchSpeller.spelled() operation more like:

F<Unspelled> -> F<Spelled>

Where each of the left column elements would map to right column elements?

I am starting to catch your drift, I think. And I like it. However, could this be a generalization we make after we make some more concrete progress?

[jsbean]

See #168 (proposal to add Unspelled and Spelled protocols).

[bwetherfield]

Yeah - this idea can definitely wait! Using octaves is good as a workaround for now!

[jsbean]

I think we should keep the current design as is for now, as it is in some sense the most information-preserving version.

In the self-pivot test, I would perhaps change the input Pitch to a legit Pitch (rather than a Pitch.Class in Pitch clothing).

Perhaps this is testing too much at once, but perhaps the self-pivot test could be implemented as:

for letterName in LetterName.allCases {
    let octave = Array(0..<11).randomElement()!
    let pitchSpeller = PitchSpeller(pitches: [0: Pitch(letterName.pitchClass + octave)], parsimonyPivot: Pitch.Spelling(letterName))
    let result = pitchSpeller.spell()
    let expected: [Int: SpelledPitch] = [0: SpelledPitch(Pitch.Spelling(letterName), octave)]
    XCTAssertEqual(result, expected)
}

[jsbean]

(Off-topic) It would probably be nice to create a composable random generator for Pitch, Pitch.Class, Pitch.Spelling, and SpelledPitch values for QuickCheck-like testing.

[jsbean]

I'm gonna close this for now. But let's keep #168 on deck!