Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON: Encoding rests #294

Closed
adrianholovaty opened this issue Jun 1, 2023 · 8 comments
Closed

JSON: Encoding rests #294

adrianholovaty opened this issue Jun 1, 2023 · 8 comments

Comments

@adrianholovaty
Copy link
Contributor

How should rests be encoded in JSON? More precisely, how can we achieve the following two goals:

  • Encode the fact that an event is a rest.
  • Provide a way to attach metadata to the rest (its vertical position, its style/color, etc.).

In the prior (XML) version of MNX, this was encoded via a <rest> element within the event.

In my initial migration from XML to JSON, I opted to use an optional "rest" key in the event, with the value being an object. See the example Three-note chord and half rest for a full document.

{
   "type": "event",
   "rest": {},
   "value": "/2"
}

Using an object for "rest" — as opposed to the simpler "rest": true — gives us a way to attach the extra rest metadata. But this is a bit inelegant, because the common case (simply encoding that something is a rest, without any additional metadata) is represented by an empty object. For programming languages in which an empty object is "falsy," this situation makes it a bit too easy for consuming code to overlook rests. (Counterpoint: rests are such a fundamental part of music that developers would detect this bug early!)

I went ahead and took this approach, for the sake of having something in the migration, but I'd like to get some community input about whether this is a good long-term encoding.

Some other options could be:

  1. Use two separate keys: a boolean saying it's a rest and a separate option with rest metadata.
{
    "type": "event",
    "rest": true,
    "restData": {"position": "B5"}
}

Downside: this structurally enables "restData" without "rest": true — and we should try to design things so that the structure avoids ambiguous or nonsensical situations if possible.

  1. Allow two separate values: "rest": true for the common case and "rest": {...} if there's additional metadata.

Downside: this means a single key might have different data types, meaning parsing is slightly more complex.

Thoughts, or other ideas?

@adrianholovaty
Copy link
Contributor Author

Issue #296 is related — it also concerns empty objects.

@samuelbradshaw
Copy link

samuelbradshaw commented Jun 1, 2023

Should there be a subType property?
"type": "event",
"subType": "rest"

Or more simply, could it be:
"type": "rest" // or "note", or "chord" (are there any other event types?)

@mscuthbert
Copy link

Not a big fan of subtypes. If it's needed then it's a new object with a new type and content:

"type": "event"
"content": {
    "type": "rest",
}

see discussion at #295 -- everything is interrelated :-)

@mscuthbert
Copy link

(Counterpoint: rests are such a fundamental part of music that developers would detect this bug early!)

Yeah, that was my thought -- in Python empty objects are falsy, same as XML and this gets me all the time in parsing MusicXML, where if note.dot: do_something() is False whether a <dot/> is present or not, since .dot will either be None or an object that evaluates to False.

However, I expect that Python and other languages that treat {} as falsey will be languages that convert anything of "type": "rest" to a typed object so that methods can be applied to them. In that case, once parsed by an appropriate JSON converter, they would become truthy instead.

@paul-bayleaf
Copy link

Re empty objects. Having objects in which missing members have default values is very reasonable. Therefore an empty object with all members defaulted is equally reasonable, and a clear and succinct notation - not in the least inelegant. RFC8259 specifically allows empty objects.

Re "rest": true for the common case and "rest": {...}. I don't like this at all, it conflates two different JSON types. Might tempt some to use both in the same object - breaking the "SHOULD be unique" condition in RFC8259.

Re Python: please don't be distracted by what any particular programming language does, or does not, do with JSON. MNX must be independent of any particular JSON implementation.

@mscuthbert
Copy link

agree with everything except:

Re Python: please don't be distracted by what any particular programming language does, or does not, do with JSON. MNX must be independent of any particular JSON implementation.

strongly disagree w/ this. If MNX doesn't work comfortably with the most popular programming languages of today (which the most popular programming languages of tomorrow will be built upon) then there's no point in it. If we didn't care about ease of use, we'd be writing the file in mnx-on our own specialized format that is used only by and for MNX and wouldn't have any of the problems of either JSON or XML. The point of using JSON in general is to tap into the ecosystem of fast and widely used implementations for parsing, writing, and manipulating it, so all our choices need to think about the ecosystems they'll be used in.

As Michael Good has said many times in the past, he knew the MusicXML had to be compatible with the syntax of either Finale or Sibelius or it'd be just another dead experimental format. Not that Finale or Sibelius should define how we think about music notation, but without industry-leader support, a format is just a thought experiment. MNX has to think about Javascript/TypeScript and Python and at least one of C++/Java in its development cycle (along with Finale, MuseScore, Dorico, Noteflight etc. and scanners like SmartScore/SharpEye, toolkits like my music21, etc.) or it too will be a thought experiment.

@adrianholovaty
Copy link
Contributor Author

We just discussed this at today's co-chairs meeting, and we decided the current behavior ("rest" as an empty object) is the best solution. We'll open a new issue regarding encoding a rest's vertical position — it would be an optional attribute in that "rest" object.

@mscuthbert
Copy link

Following from #315 -- I suggest that the position optional attribute be measured in the rest object in terms of the staff positions mentioned there, independent of clef. I also think that we should be able to note whether the position is important/specified or generated, so consuming applications that use MNX as an internal format can distinguish between whether they positioned a rest on a certain line because that's what worked (with other voices/number of staff lines/etc.) or because the user positioned it there. Such distinctions should be maintained in multiple places in MNX (accidental display, etc.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants