rism-digital / verovio

🎵 Music notation engraving library for MEI with MusicXML and Humdrum support and various toolkits (JavaScript, Python)
https://www.verovio.org
GNU Lesser General Public License v3.0
685 stars 185 forks source link

Toolkit methods getHumdrum and getHumdrumFile is not correctly named #3827

Open ahankinson opened 1 month ago

ahankinson commented 1 month ago

The get* methods tend to return a value from the internal state of the toolkit, while the render* methods will render the toolkit data to a file. So getHumdrumFile(filename: str) -> bool should probably be renamed to renderToHumdrumFile(filename: str) -> bool to match.

Additionally, getHumdrum() -> str should probably be renamed renderToHumdrum() -> str

craigsapp commented 1 month ago

I am thinking about this: The output of getHumdrum() is more than renderToHumdrum(). Usually it is used to capture an intermediate state when loading Humdrum data on its way to conversion into MEI (rather than conversion from MEI to Humdrum). I am traveling to Europe tomorrow and will have jetlag and conferences after that so remind me in a few weeks if I forget too look into this.

ahankinson commented 1 month ago

Usually it is used to capture an intermediate state when loading Humdrum data on its way to conversion into MEI (rather than conversion from MEI to Humdrum).

OK; what is getHumdrumBuffer for? That seems like it's also a place to get humdrum data, possibly not as a conversion?

craigsapp commented 1 month ago

Yes, that is something to check related to names of output functions (these were all added 8 years ago, so I will have to read the code to see what they do and how to normalized their names).

lpugin commented 1 month ago

GetHumdrumBuffer is the equivalent of GetCString (and SetCString). I am not sure why Humdrum has a dedicated function. @craigsapp wouldn't be possible to use Set/GetCString instead and to drop them?

craigsapp commented 1 month ago

Here is a schematic of import of Humdrum into verovio (names of variable/functions may be approximate):

Screenshot 2024-10-14 at 04 42 50

I need access to the intermediate Humdrum data after it has been transformed and before it is converted to MEI. In this case renderToHumdrum is misleading. Command line: verovio file.krn -t humdrum gives the intermediate version of the Humdrum data before being processed by iohumdrum.cpp.

For conversion of MEI to Humdrum, the schematic is (approximately, but likely):

Screenshot 2024-10-14 at 04 48 33

This is the case where it can be renamed appropriately to renderToHumdrum.

Here is the mei2hum converter: https://github.com/craigsapp/humlib/blob/master/src/tool-mei2hum.cpp

lpugin commented 1 month ago

OK. My understanding would be that we can:

Does it sound right?

craigsapp commented 1 month ago

Yes that sounds about right. SetHumdrumBuffer should only be needed internally in verovio (in Toolkit class, I would have to check how InputHumdrum class interacts with it). The old names should be kept for a while with deprecation warnings (in Javascript version of verovio).