ELVIS-Project / vis-framework

Thoroughly modern symbolic musical data analysis suite.
http://elvisproject.ca/
31 stars 6 forks source link

Generalize .run() in indexers? #411

Closed alexandermorgan closed 7 years ago

alexandermorgan commented 7 years ago

In VIS 3 multiple indexers may be able to share the same .run() method which they could inherit from indexer.py if it were there. I'm not yet sure how many could make use of this, so I'm not sure if it's worth changing. I should know in the next week or two.

crantila commented 7 years ago

There are several things that could be moved to the Indexer superclass, so this seems like a good idea. Not sure I understand exactly what you're thinking—it's that you want to put a generic run() implementation in Indexer, right? One that would use the indexer_func, for example?

alexandermorgan commented 7 years ago

Sorry for not being clearer. Yes, I was thinking of putting a generic run() in Indexer.py. It would use dataframe.applymap(indexer_func) to do the indexing necessary. There are several cases where this would not be sufficient but even if it's only inherited by one or two indexers it could be nice to have this there, especially to facilitate the writing of future indexers. I'm not certain I'll have time for this for VIS 3 but it would be nice.

alexandermorgan commented 7 years ago

This is addressed in alex_devel starting from commit 894e4ac987c90a758fbe6c5fcb61e11daedc0c98 (and the one just before it on indexer.py). .run() has been removed from the noterest, beatstrength, and measure indexers. Later it will be removed from the fermata indexer as well. Depending how (and especially if) the timesignature, lyric, beat, and dynamics indexers get written, they could all potentially make use of this inherited .run() method in indexer.py.