JuliaMusic / MusicXML.jl

MusicXML in Julia
MIT License
13 stars 7 forks source link

Name of the types? #31

Closed aminya closed 4 years ago

aminya commented 4 years ago

We already named Note type as NoteX type to prevent conflict with MIDI.jl. Now, I remembered that Time type is also available in the Dates package, which is a standard library in Julia, and one cannot use these two together.

I am wondering if I shouldn't export the types at all and instead introduce an alias for the package (like how I did inside IntelVectorMath.jl):

export MX
const MX= MusicXML

which allows calling the types like:

MX.Note
MX.Time

The alias can be different things like MX, Mx, mx, MXL, mxl, Mxl etc.

In addition, I can also write a macro that translates every call to MusicXML's types and add to MusicXML. them.

For example:

@MX begin
  Note(something)
  Time(something)
end

will be translated to:

  MX.Note(something)
  MX.Time(something)

People can easily mix different types from different libraries by using either () or begin end

This also allows me to define MIDI version of the types inside MIDI.jl or some other package like MetaMidi.jl

aminya commented 4 years ago

@Datseris what do you think?

Datseris commented 4 years ago

Gimme a couple of days because things are going crazy at work, and I'll have a detailed look.

Datseris commented 4 years ago

Just a quick note before I have a detailed look, and also because your reply is necessary: Why should you prefer the complex @MX macro, and/or prefixing everything with MX., instead of de-facto prefacing all (exported) things with MX in code, e.g. why not just define and use internally directly things like MXNote, MXTime. It may make things clearer in the long run? You could still have aliases if you really want to use exclusively Note in code by adding const MXNote = Note, but since two letters are not a big deal to type, I would generally go for just using MXNote/MXTime straight-away, and not even defining Note/Time.

aminya commented 4 years ago

Just a quick note before I have a detailed look, and also because your reply is necessary: Why should you prefer the complex @MX macro, and/or prefixing everything with MX., instead of de-facto prefacing all (exported) things with MX in code, e.g. why not just define and use internally directly things like MXNote, MXTime. It may make things clearer in the long run? You could still have aliases if you really want to use exclusively Note in code by adding const MXNote = Note, but since two letters are not a big deal to type, I would generally go for just using MXNote/MXTime straight-away, and not even defining Note/Time.

In terms of typing MX.Note and MXNote are similar (just a . difference), but MX.Note is much clearer to me, so I prefer MX.Note over MXNote.

The macro isn't anything complex. It just adds MX. to the beginning of the types. See https://github.com/JuliaMusic/MusicXML.jl#creating-example

Using the types is only needed if someone wants to create MusicXML. For parsing these are not needed anyway.

I am going to add another macro that imports all the MusicXML types if someone doesn't use anything else.

@importMX # imports all the MusicXML types.
Datseris commented 4 years ago

I'll give you my take on it based on my experience developing packages so far, and you can take it or leave it. Even if you don't take it, at least spend some time thinking about it (since I spend so much time writing it down).

  1. MXNote is better than MX.Note as far as a Julia user is concerned: (a) names are prefaced from their parent module if they are to be extended via multiple dispatch or (b) if they are not exported from the module and thus not part of the public API. None of these two cases is true here. (c) A third case where you preface a name by its parent module is when you don't want to access the public API (defined by the module's export statements), and you instead do import MusicXML and thus explicitly access any name you want as MusicXML.Note. This is not the case here either, since to the best of my knowledge your Note is central and you want it to be part of the public API.
  2. MX.Note is objectively less clear. Why? MXNote is a single Julia name. That's it, there is no further discussion, it is a Julia type that you use. MX.Note is prefaced with MX. Why is that? What is MX? It is not MusicXML, because they are not the same name. Oh wait, they are the same, because there is an alias... But why should I use MX.Note instead of just Note? Why should Note note be exported? All of these confusing questions exist for MX.Note, but not MXNote. Clarity is not about how many characters you type on your keyboard.
  3. Macros are always complex (they are textual code manipulation de facto) and you should avoid them, especially when the same result can be achieved without macros. This is not my opinion but the opinion of pretty much every senior Julia developer. Please see this talk: https://www.youtube.com/watch?v=mSgXWpvQEHE&t=2839s (minute 10). In addition, you might be very familiar with macros etc., but take a moment to think how much of the Julia user-base is really familiar with macros.
  4. What happens if in the example you link I want to use both MIDI.Note and MusicXML.Note? the example collapses.
  5. You just added another macro to the mix, and it seems to me you overcomplicate the situation without much benefit. MusicXML seems to me like a Julia package with a small and specific scope, why would it have such a massively different behavior with respect to public API? Like, you wrap your entire usage of MusicXML in a macro? This doesn't make much sense. For all other Julia packages that exist, the way the public API is defined is set in stone: you export the names for the public. Think about it for a moment and think whether MusicXML.jl should have a completely different way to create a public a API that no other Julia package has done so far. Have you seen this behavior anywhere else? If not, there might be a reason.

So, as far as I can tell, all the complications I mention above become trivially resolved if (1) you don't export Note and require explicit qualification, or simply (2) you use MXNote or NoteX or whatever other nonconflicting name, and just define the alias const Note = MXNote to use inside your code (as for some reason you seem to be hell-bend on using exactly the name Note character-for-character, something that I don't understand why). Same can be repeated for Time and MXTime. Again I don't understand why you would want to use an already existing name Time character-for-character.

aminya commented 4 years ago

I'll give you my take on it based on my experience developing packages so far, and you can take it or leave it. Even if you don't take it, at least spend some time thinking about it (since I spend so much time writing it down).

1. `MXNote` is better than `MX.Note` as far as a Julia user is concerned: (a) names are prefaced from their parent module if they are to be extended via multiple dispatch or (b) if they are not exported from the module and thus not part of the public API. None of these two cases is true here. (c) A third case where you preface a name by its parent module is when you don't want to access the public API (defined by the module's `export` statements), and you instead do `import MusicXML` and thus explicitly access any name you want as `MusicXML.Note`. This is not the case here either, since to the best of my knowledge your `Note` is central and you want it to be part of the public API.

2. `MX.Note` is objectively less clear. Why? `MXNote` is a single Julia name. That's it, there is no further discussion, it is a Julia type that you use. `MX.Note` is prefaced with `MX`. Why is that? What is `MX`? It is not `MusicXML`, because they are _not_ the same name. Oh wait, they are the same, because there is an alias... But why should I use `MX.Note` instead of just `Note`? Why should `Note` note be exported? All of these confusing questions exist for `MX.Note`, but not `MXNote`. **Clarity is not about how many characters you type on your keyboard**.

3. Macros are always complex (they are textual code manipulation de facto) and you should avoid them, especially when the same result can be achieved without macros. This is not my opinion but the opinion of pretty much every senior Julia developer. Please see this talk: https://www.youtube.com/watch?v=mSgXWpvQEHE&t=2839s (minute 10). In addition, you might be very familiar with macros etc., but take a moment to think how much of the Julia user-base is really familiar with macros.

4. What happens if in the example you link I want to use both `MIDI.Note` and `MusicXML.Note`? the example collapses.

5. You just added another macro to the mix, and it seems to me you overcomplicate the situation without much benefit. MusicXML seems to me like a Julia package with a small and specific scope, why would it have such a massively different behavior with respect to public API? Like, you wrap your entire usage of MusicXML in a macro? This doesn't make much sense. For all other Julia packages that exist, the way the public API is defined is set in stone: you `export` the names for the public. Think about it for a moment and think whether MusicXML.jl should have a completely different way to create a public a API that no other Julia package has done so far. Have you seen this behavior anywhere else? If not, there might be a reason.

So, as far as I can tell, all the complications I mention above become trivially resolved if (1) you don't export Note and require explicit qualification, or simply (2) you use MXNote or NoteX or whatever other nonconflicting name, and just define the alias const Note = MXNote to use inside your code (as for some reason you seem to be hell-bend on using exactly the name Note character-for-character, something that I don't understand why). Same can be repeated for Time and MXTime. Again I don't understand why you would want to use an already existing name Time character-for-character.

The first reason is because these types are MusicXML's types. All the types have the same name in the original MusicXML: https://usermanuals.musicxml.com/MusicXML/Content/EL-MusicXML.htm

The second reason is that there is no point in changing my type names just because there are similar type names in other packages.

So I don't want to change the name of the types just for the sake of Julia's usage.

I wanted to export all the types at first, but because of the conflicts, I am skeptical to do this. Although, macros seem scary they are not. I agree with the video you sent, but remember that these macros don't do anything special. That video is about abusing them. We don't want to remove a language's feature (C/C++ also use macros a lot.).

@importMX just does: import MusicXML: ScorePartwise, Part, Measure, Note, Chord, Unpitched, Rest, Pitch, Attributes, Time, Transpose, Clef, Key, PartList, ScorePart, MidiInstrument, MidiDevice, ScoreInstrument, but in a more compact way/

@MX just adds MX. to the name of the types if they are inside the macro territory. This is just a macro to save typing MX. for every Type.

Mixed usage is very easy:

@MX begin
    Note(stuff) # becomes MX.Note
    MIDI.Note(other_stuff) # remains untouched
end

or

@MX(Note(stuff)) == Note(other_stuff)
# translates to: MX.Note(stuff) == Note(other_stuff)

I have seen these kinds of stuff in other packages too: