Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2329)

Issue 559440043: Fix SyntaxWarning's (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 2 months ago by hahnjo
Modified:
4 years, 2 months ago
Reviewers:
dak
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Fix SyntaxWarning's Individual changes: 1) abc2ly: Fix SyntaxWarning This line still works in Python 3.8, but it throws a warning: > SyntaxWarning: "is" with a literal. Did you mean "=="? 2) musicxml2ly: Remove drummode This code is fundamentally broken: NoteEvent::ly_expression() and NoteEvent::chord_element_ly() miss a '%' to format the string. Instead the code "calls" the string which Python meets with a SyntaxWarning. Trying to hit this code path, musicxml only sets the drum_type attribute in Note::initialize_drum_event() called from Note::to_lily_object(). However as far as I understand the MusicXML specification, a <note> must always have either a <pitch>, or <unpitched>, or <rest>. If I try to construct an (invalid) input with only an instrument name, musicxml2ly breaks at another function, so this is really dead code.

Patch Set 1 #

Patch Set 2 : Drop drumtype_dict #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -60 lines) Patch
M python/musicexp.py View 5 chunks +0 lines, -10 lines 0 comments Download
M python/musicxml.py View 1 3 chunks +0 lines, -23 lines 0 comments Download
M scripts/abc2ly.py View 1 chunk +1 line, -1 line 0 comments Download
M scripts/musicxml2ly.py View 1 5 chunks +0 lines, -26 lines 0 comments Download

Messages

Total messages: 3
dak
> However as far as I understand the MusicXML specification, a <note> must always have ...
4 years, 2 months ago (2020-02-02 15:44:48 UTC) #1
hahnjo
On 2020/02/02 15:44:48, dak wrote: > > However as far as I understand the MusicXML ...
4 years, 2 months ago (2020-02-02 15:47:44 UTC) #2
hahnjo
4 years, 2 months ago (2020-02-02 15:49:36 UTC) #3
Drop drumtype_dict
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b