leonawicz / tabr

R package: tabr. Notation-based and tidy music data analysis and transcription.
https://leonawicz.github.io/tabr/
Other
128 stars 10 forks source link

Volta renders wrong #22

Closed kimo-k closed 5 years ago

kimo-k commented 5 years ago
> x %>% volta(2,c(e1,e2))
<Musical phrase>
\repeat volta 3 { <c\5>4 <e\4 c'\3 g'\2>4 <e\4 c'\3 g'\2>2 | }
\alternative {
  { <a\5>1 <b\5>1 | }
}

Looks like the extra set of brackets in \alternative are causing the volta to render improperly

leonawicz commented 5 years ago

Ah, okay. See the help docs for volta. endings takes a list type object. list(e1,e2) vs. c(e1,e2).

> volta(p("c ec'g' ec'g'", "4 4 2", "5 432 432"), n = 2, endings = list(p("a", 1, 5), p("b", 1, 5)))

The extra curly braces are proper: http://lilypond.org/doc/v2.18/Documentation/notation/long-repeats

Using c worked in this case before (unintended) but no longer will and isn't meant to. In general phrase objects always needed to be listed with list to avoid coercing them to strings. Previously, using c for basic concatenation fell back on the default method of coercing them to simple character strings (an atomic vector, unlike a list). That just happened to be what this function was going to do to those endings phrases anyway so it slipped through.

Now that a custom method has been written for c for the phrase class (it binds them together into a longer phrase object), you have to use the argument as intended to make a list of that object type. In R help docs list means list type and not an atomic vector like character or numeric. The addition of the c.phrase method is one of those breaking changes that was bound to hit some users. Probably one of the last ones there will be in the package that can have such sweeping consequences. Really, this function should have rejected the non-list before, but didn't.

These repeat functions will probably get some refining later. For example, I may remove the aggressive interpretation of basic strings as potentially valid phrases and simply require that a legit phrase object be provided. It's convenient when I just want to say "r1" but I may be giving these repeat functions a bit too much power there. In other places in the package I have been limiting which functions eagerly try to turn character to a tabr class object. Tending toward reserving that eagerness turning a simple class into a more complex one for functions that are explicitly about class coercion like as_noteworthy and a handful of others.

kimo-k commented 5 years ago

Right, I understand. Funny, I forgot how voltas work in ly. I'm excited to see the potential of these new class methods.

So far I've been doing straight transcription, but soon I may start using tabr as an educational tool, using more of the analytic capabilities.