marl / jams

A JSON Annotated Music Specification for Reproducible MIR Research
ISC License
185 stars 26 forks source link

display viz mappings: equal parts RFC and PR #177

Closed ejhumphrey closed 6 years ago

ejhumphrey commented 7 years ago

The jams.display submodule rocks but I've had to override the VIZ_MAPPING symbol table to use it with different namespaces, e.g. tag_open. I've taken the liberty of adding that one here, and alphabetizing the symbol table so we can add to it with some kind of convention going forward.

but!

this might be indicative of something we want to fix in general, and I'm curious to kick it around (here, I guess?). Should a namespace JSON file also specify how it should be visualized? seems like adding this info in two places is a design smell.

also: happy to move this to an issue first.

bmcfee commented 7 years ago

I've taken the liberty of adding that one here, and alphabetizing the symbol table so we can add to it with some kind of convention going forward.

I don't think it should be alphabetically or lexically ordered. The choice of OrderedDict was intentional, and meant to reflect priority and specificity of the different annotation viz functions when multiple matches are possible. For instance, basically everything is convertible to tag_open, but you wouldn't want to use that viz for things where more precise displays are possible.

I agree that this is not well documented though, so we should improve on that.

bmcfee commented 6 years ago

(ping @ejhumphrey) any thoughts on above?

bmcfee commented 6 years ago

@ejhumphrey can we close this one?

bmcfee commented 6 years ago

Reupping this one again: @ejhumphrey can you revert the ordering changes (with a comment to indicate that order reflects precedence of namespace coercion)? Otherwise, I think tag_open should be a low-priority viz, so its placement at the end is fine.

bmcfee commented 6 years ago

Chatted with @ejhumphrey out of band. We'll close this out and reopen as a new PR just with the added viz link.