search

ELVIS-Project / vis-framework

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

Remove Lilypond Dependencies #347

Closed ahankinson closed 8 years ago

ahankinson commented 9 years ago

I have it on good authority that the lilypond backend for music21 is being considered for deprecation. We should start removing this dependency in VIS.

alexandermorgan commented 9 years ago

This is a good idea, we should talk about what to replace it with. Perhaps Musescore, but ultimately it seems like it would be best to wait for Verovio to be ready. As I understood it, that would also require MEI output for music21. Do we know if support for LilyPond is being replaced with something else?

ahankinson commented 9 years ago

I think it's ultimately being replaced by MEI. I'd like to get MEI output from music21 happening within the next year, including support for annotations. That would also depend on Verovio support for annotations as well.

ahankinson commented 9 years ago

Thinking about this a bit more -- I don't know if we can target Musescore in the interim. For VIS we would need to do things like access the ability to annotate scores, which means exporting the annotations to MusicXML. I'm not sure music21 would do this, and I'm not sure if Musescore would read them. It might be worth a quick check to see if this is possible.

alexandermorgan commented 9 years ago

I think it's best to stay with lilypond until we replace it with a permanent solution. Even if lilypond is deprecated it should still work fine for us for a while so why don't we wait until we have a better idea of how long it's going to take Laurent to make Verovio a viable option.

crantila commented 9 years ago

I heard the same thing months ago about the fate of LilyPond in music21, and I think you've reached the most reasonable conclusion. However, there are two additional things to note: (1) MEI output is not generally a suitable replacement for LilyPond output, although it will work for VIS; and (2) VIS doesn't use music21 for LilyPond anyway, it uses outputlilypond.

crantila commented 9 years ago

Has there been any more discussion of this? My concern is pretty simple: if you feel like music21-to-MEI conversion and Verovio will be ready soon enough, I'll close these issues related to LilyPond in VIS, and these in outputlilypond. Otherwise, LilyPond is still fun for me (hey, keep down the laughter!) so I'd be happy to help work these through... if there will be an audience...

alexandermorgan commented 9 years ago

We actually don't have any specific issues producing output with LilyPond via outputlilypond. So I thought the plan was just to continue using LilyPond through outputlilypond until an alternative makes itself available. I think we can just close this issue.

ahankinson commented 8 years ago

Verovio will now read MusicXML

mrbannon commented 8 years ago

Outstanding

On Fri, Apr 1, 2016 at 4:58 PM, Reiner Krämer notifications@github.com wrote:

[image: :+1:]

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/ELVIS-Project/vis-framework/issues/347#issuecomment-204563906

crantila commented 8 years ago

It seems reasonable not to install outputlilypond by default, and to prefer VIS-Ualizer for output in many cases. But unless VIS's LilyPond components are causing problems for you, I would prefer to keep them in VIS. Just because you're not using something doesn't mean nobody else is.

crantila commented 8 years ago

Deprecated functionality is unusable because it will be removed; removed functionality can't be used because it's not there. It amounts to the same thing.

Throughout this discussion, the only justifiable reason to remove/deprecate LilyPond in VIS was based on the misunderstanding that VIS uses music21 for LilyPond output. It doesn't, so that point is irrelevant. Aside from that, the consensus has been that you don't want to use LilyPond any more, therefore you're going to remove the possibility.

Why? That doesn't make sense. Are you using too much developer time to maintain it? Are there too many bugs that can't be fixed? These are good reasons to deprecate/remove functionality. "We don't use it anymore" by itself is a poor reason to deprecate/remove functionality that still works just fine.

crantila commented 8 years ago

This isn't about LilyPond per se (though that may be the only reason I noticed in the first place). The issue is responsible framework management.

I may be missing something, but so far this looks like busywork. Andrew's initial post was well-justified but that's been cleared up. The issue remained open during discussion of what should replace LilyPond in the web front-end where LilyPond works poorly. The only technical reason to remove VIS's LilyPond functionality was resolved, so why did this discussion start up again?

What I believe you need, which you haven't yet provided, is a description of the technical problems you plan to solve when you break the Framework's public API by removing a series of modules that are several years old. Are the LilyPond analyzers causing a maintenance burden? Are you having problems configuring VIS with Rodan because of outputlilypond? Is it the extra ~25KB of disk space that's causing problems? So far you've given emotional appeals ("I used LilyPond on my dissertation") and buzzwords ("VIS should be as lean as possible") but these hold no technical merit. The fact you're not using this functionality is relevant, but is it sufficient justification for breaking the public API?

According to PyPI, vis-framework v2.4.1 alone has been downloaded more than 1300 times in less than a month. Some of those downloads were by real people, and we have no idea how they're using VIS. If those people happen to see the deprecation notice in VIS 3, what are they supposed to do? Are you going to make sure there's an alternative for them? Are you going to write and maintain the extension you proposed? If so, you're making their lives more heavyweight, harder to understand, and harder to maintain by adding an extra dependency. If not, you're abandoning an unknown number of your users by removing years-old functionality from the API.

If there were any benefit to doing this, if there were a problem that you could solve by making the proposed change, the issue would be very different. Just because the core developers don't use something doesn't mean nobody is using it. Just because a feature is a bad match for the core developers' use case doesn't mean it's a bad match for anybody else.

I'm open to further discussion, but given the information presented so far, I object to removing VIS's LilyPond functionality. I believe I've made my reasons for dissent sufficiently clear, so I'm prepared to step back at this point and let you handle the situation.

ahankinson commented 8 years ago

AFAIK, VIS has just two core dependencies: music21 and pandas. Both of these operate on the level of data.

We are moving towards data-only output for VIS, and removing the dependencies on 'desktop' applications (like matplotlib) that limit its usefulness in networked and data processing contexts. The move to D3.js and the vis-ualizer module reflects this type of direction, where the visualization of the data has been moved to a "third-party" module, freeing up the core of vis to just focus on data input, manipulation, and output.

The output of lilypond is PDF, which is tangential to the goal of data-only output, and few (if any) other systems accept, or plan to accept, the lilypond file format as an interchange format. This means that for the forseeable future, lilypond output represents limited usefulness to other types of renderers and analysis systems (Musescore, Verovio, etc.). A separate "third-party" module, i.e., outputlilypond should still be the preferred option.

Put another way, we will not have a core module for Verovio, or a core module for MuseScore, so why have a core module for LilyPond?

I think it would be better to focus our energies on creating interoperable data outputs that can be fed to other tools. We get MusicXML "for free" from music21; we should focus on making MEI better, certainly. But I find it hard to justify a core dependency on lilypond. Wouldn't you agree?

crantila commented 8 years ago

From the perspective of your long-standing development goals for VIS, and maintaining consistency in the API, I can see how this makes sense, but it still seems like a mindset problem of "we don't use it so it has to go." You write as though VIS's LilyPond functionality is holding you back, costing a lot of developer time, and preventing you from future enhancements. You write as though I'm suggesting adding new functionality where none existed, and you'll have to maintain it. As far as I'm aware, none of that is true.

VIS has an Experimenter to run LilyPond the program. There is no existing or proposed "core dependency on lilypond." All the other LilyPond analyzers operate within the realm of pandas and music21 and surely count as interoperable data outputs. Many of these analyzers could find use for things other than LilyPond, like the module that outputs music21 notes from pandas data. We're only talking about 409 source lines of code between the two modules—it's not a terrible burden.

If you're thinking of outputlilypond, this is already hosted in another repository where I'm the maintainer. I've thought about releasing it separately many times, and I've offered to fix any issues as well. Would you like me to remove the outputlilypond submodule from the VIS repository, and release it separately on PyPI so it can be installed with VIS only when required?

I recognize that API consistency helps people understand frameworks more easily, and I'll surely admit to making a host of bad decisions in VIS that you have to clean up now. Again, if we were starting from scratch and talking about whether to write new modules for LilyPond output, this would be a totally different issue. As it is, you still haven't named a specific problem that you face, and (as far as I'm aware) the LilyPond analyzers are consistent with the other analyzers.

Whatever your decision, you should at least recognize that scrapping a desktop-only output in favour of a web-only output is not a replacement. Consider it from my perspective: the only reason I use VIS is because of its unique LilyPond capabilities. Suddenly my use case is deprecated because the core developers aren't using it. Are these web-only replacements really going to do it for me? Suddenly VIS becomes a huge pain, I have to rewrite my analysis programs and I don't get any benefit for it, and every time I want to see score output I have to start up a web server and browser. For me, I'll probably just stop updating my personal fork of VIS. For others, what will they do? I know you've all faced similar situations before—how did you feel and what did you do about it? Is the small gain in intellectual consistency of VIS's output formats worth alienating an unknown number of users?

ahankinson commented 8 years ago

If you're thinking of outputlilypond, this is already hosted in another repository where I'm the maintainer. I've thought about releasing it separately many times, and I've offered to fix any issues as well. Would you like me to remove the outputlilypond submodule from the VIS repository, and release it separately on PyPI so it can be installed with VIS only when required?

Yes, I think that's the idea.

Consider it from my perspective: the only reason I use VIS is because of its unique LilyPond capabilities. Suddenly my use case is deprecated because the core developers aren't using it.

I don't think that's fair. We haven't deprecated your use case, we're only moving lilypond functionality out into a separate component. The fact that outputlilypond is already a submodule should make this a fairly obvious change, since it's already decoupled; a process which you yourself started. And I'm fairly sure that Alex (who is a 'core dev') still uses the lilypond output to render the score annotations.

You are in a much better position than us to write and support the lilypond module, and we're fully recognizing this and welcoming your input. The tone of your messages seems to indicate that this is somehow a malicious decision, but this is not the case.

As far as I can tell the entire substance of the change is that two files will move from vis to the 'outputlilypond' project. In return you get the freedom to make any and all design decisions on the nature and substance of the lilypond output and interaction, including desktop rendering if you like.

You also seem to think that we're putting 'web-based' things into VIS, but as Reiner mentioned the opposite is the case; our web-based tools are going into our own external modules. You will not need to "fire up a web server" (unless you want to).

crantila commented 8 years ago

The tone of your messages seems to indicate that this is somehow a malicious decision, but this is not the case.

I know you don't intend to be hostile, and I know we're all interested in the best outcome for VIS. That's why we're digging in. What I see is answers for questions I'm not asking, and my actual questions being ignored, so there's a communication failure. I think we got caught up in rhetoric and we're not even thinking on the same level, so let's take a step back and try a different approach.

I'm going to state what I believe is your proposal and reasons, and then my own counter to that. Once we agree on what the perspectives represent, we can address details meaningfully, and only as required.

Proposal: remove outputlilypond, indexers.lilypond, and experimenters.lilypond from VIS.

Justification: LilyPond output capability is inconsistent with the VIS Framework's long-term development goals. Data visualization preparation should happen outside the Framework, in purpose-built extensions, while the Framework should only produce interoperable data as output.

Counterproposal: remove outputlilypond from VIS, but leave indexers.lilypond, and experimenters.lilypond (minus the capability to run LilyPond the program).

Justification: By removing outputlilypond, all the LilyPond-only functionality is gone too. The remaining analyzers all produce interoperable data, and some of them may be useful for things other than LilyPond output, potentially with modifications. This restores consistency to the Framework's architecture while avoiding any deprecations or backward-incompatible API changes.

crantila commented 8 years ago

(By the way, thank you for actually having this discussion, I do appreciate that you're still taking my input into consideration).

ahankinson commented 8 years ago

Counterproposal: remove outputlilypond from VIS, but leave indexers.lilypond, and experimenters.lilypond (minus the capability to run LilyPond the program).

Won't that introduce code that will automatically fail if run without the 'optional' outputlilypond module? As far as I can see, those bits depend on outputlilypond, so we're not actually solving anything. Am I missing something?

crantila commented 8 years ago

Won't that introduce code that will automatically fail if run without the 'optional' outputlilypond module?

As proposed, yes. The failure would only affect people who are about to run outputlilypond anyway, so I was thinking that an informative error message is a reasonable trade-off. This implies a bit of refactoring but it could be accomplished without breaking the public API.

An alternative is to remove the LilyPondExperimenter entirely, which runs both outputlilypond and lilypond, and does nothing else. This introduces a backward-incompatible change, but a small one. Because the "run outputlilypond then lilypond" task could reasonably be held in a script in outputlilypond, this amendment also leaves a clear solution for users affected by the change.

ahankinson commented 8 years ago

I'm still a bit unclear on what you're proposing, though. What I'm proposing is that we have a package vis-lilypond which packages up the existing experimenter and indexer along with the 'guts' of lilypond output for vis. This seems to me to be what you're now getting around to, so maybe we're on the same page now?

With everything packaged together there is no need for an informative error message: If you want it, you have it; if you don't want it, there's no indication that anything is wrong. I was always taught that good software design is informing the user of an error, but better software design is ensuring the error doesn't occur in the first place.

ahankinson commented 8 years ago

@crantila ping?

crantila commented 8 years ago

Sorry for the delay. When we got this mostly sorted out I needed to put it down because everything is on fire!

Your idea isn't quite what I had in mind. I would prefer to keep the LilyPond-related indexers and experimenters in VIS itself. If we're looking at an ideal architecture for VIS, maybe they shouldn't be there, but that's sort of beside the point.

My ideal plan is:

... here are additional reasons I don't prefer moving the LilyPond analyzers to a plugin:

To be clear, I'm not asking you to maintain the LilyPond analyzers for me. I'm asking to keep them in VIS itself so that you know as soon as they break and you can warn me. The extra half-second it takes to run the test suite there is nothing compared to the time it takes me to manage an additional package.

ahankinson commented 8 years ago

This:

If the analyzers are kept out-of-tree, it's my responsibility to make sure they work with the latest versions of VIS, and to document which versions of the analyzers are compatible with which versions of VIS. I can't commit to doing that reliably, and it's an extra dependency challenge for users to take on.

and this:

They're already in VIS, and again I may be wrong, but I don't believe they're causing problems for you, so why change something that works? It seems like a minimal advantage for anyone, and a burden for users and for me.

are mutually exclusive. There can't be no cost to maintaining it, while we continue to try and find someone to maintain it. There is a maintenance cost, especially as we move towards 3.0. We are not in the best position to do this, since very few of us use the LilyPond capabilities, so you, as the principle and expert user for this, are best positioned to take this on.

However, that's enough from me. I've just spoken with @alexandermorgan, and think that he should have the final say on this issue since he is currently the principle developer and is actually the person responsible for maintaining this (or not).

crantila commented 8 years ago

Sure, it doesn't make sense if you omit the context. It's about the comparative effort of the two alternatives. I'm asking you to file a GitHub issue when a LilyPond module breaks, and then I'll fix it. It takes you no additional effort to run the tests, so your whole maintenance effort is filing an issue. Yes, I still have to fix it, but the combined effort there is much lower than the effort of maintaining a separate package.

I'm still missing an explanation of the techincal problems you're trying to solve. There are two nontechnical problems I can see: that LilyPond analyzers don't fit your long-term vision for the Framework; and that you anticipate a large maintenance burden. Why do you anticipate a lot of maintenance work? Have you sunk a lot of effort here already? (The commit history answers "no.") Are you trying to make big changes but the LilyPond analyzers are holding you back? (If so, why didn't you mention it when I asked in previous posts?)

So this looks like a problem of intellectual consistency, which again I do understand is important. But you're asking me to take on sole maintainership of this code, and our users to refactor their code—and there's no advantage for us. That's why I believe the consistency argument is insufficient in this case.

I guess we'll see what Alex says.

alexandermorgan commented 8 years ago

After reviewing this issue I'd like to spend max. 5 minutes in the lab meeting on Friday on the question of whether score output is actually a visualization. The case for it being a visualization is obvious, but a score is fundamentally different from a bar chart or other visualizations. I see it as a human-readable spreadsheet, and so being able to output a score is sort of like being able to export data structures to a csv. Score output has been the only way that people have been able to check any of our ground-truth analyses so it's pretty central to using VIS for research, though admittedly not central computationally.

After this discussion on Friday we will either: 1) Remove outputlilypond as a core dependency in VIS 3.0 (it may not technically be a core dependency, but to the best of my knowledge you can't run VIS without it at the moment); or 2) do the above and also deprecate all other Lilypond-related components in VIS 3.0

crantila commented 8 years ago

Well that's a perspective I wasn't expecting...

alexandermorgan commented 8 years ago

I didn't end up doing this for one because I didn't have the time, but more importantly because I think VIS desperately needs a way of producing score output. A version of VIS that only produces data would make sense for an online app that has a dedicated plug-in already in place that produces score output (like Verovio). But until VIS in Rodan is more developed the focus of VIS development should stay on the desktop version and I think a desktop music analysis program makes little sense without score output of some kind.

musicus commented 8 years ago

We don't have an alternative right now.

alexandermorgan commented 8 years ago

All LilyPond dependencies have been removed as of version 3.0.3. I still think that VIS needs a means of producing score output with the option of adding annotations. I removed these dependencies because outputlilypond is not up to date and between moving to Python 3, and music21 2.1.2, it has a few small but blocking issues. If we can get outputlilypond working again, or if we can use some other score-producing library, then I think that should be a big priority. I don't have any solutions for the immediate future though. Also, if we do ever get score output back, it should be one indexer (not an experimenter since scores are time-aligned) that can optionally take commands for annotations, rather than having several analyzers in a non-automatic workflow.