Named note #65
Named note #65
Conversation
src/namednote.jl
Outdated
| """ | ||
| mutable struct NamedNote <: AbstractNote | ||
| name::String | ||
| pitch::UInt8 |
There was a problem hiding this comment.
Pitch shouldn't exist. If you ever need it to compute equallity, compute it on the spot from name.
Project.toml
Outdated
| @@ -1,10 +1,11 @@ | |||
| name = "MusicManipulations" | |||
| uuid = "274955c0-c284-5bf7-b122-5ecd51c559de" | |||
| repo = "https://github.com/JuliaMusic/MusicManipulations.jl.git" | |||
| version = "1.6.3" | |||
| version = "1.6.4" | |||
src/namednote.jl
Outdated
|
|
||
| NamedNotes(notes_string::String; tpq::Int = 960) = Notes([NamedNote(String(s)) for s in split(notes_string," ")], tpq) | ||
|
|
||
| Note(n::NamedNote) = Note(n.pitch, n.position, n.velocity, n.duration, n.channel) |
There was a problem hiding this comment.
same; don't use pitch, compute it on the spot.
|
I can't finish the test since I don't have a macos device. |
| NamedNote(n::Note; pitch_name::String = "") = | ||
| length(pitch_name) == 0 ? NamedNote(pitch_to_name(n.pitch), n.position, n.velocity, n.duration, n.channel) : NamedNote(name, n.position, n.velocity, n.duration, n.channel) | ||
|
|
||
| NamedNotes(notes_string::String; tpq::Int = 960) = Notes([NamedNote(String(s)) for s in split(notes_string," ")], tpq) |
There was a problem hiding this comment.
This function does not have a documentation string!
test/namednote_test.jl
Outdated
| cn = Note("C#4") | ||
| nn3 = NamedNote(cn) | ||
| @test nn3.name == "C♯4" |
There was a problem hiding this comment.
This test is unecessary, you already do the same test in the start of the test suite. Instead, write a test that compares a NamedNote with a Note generated from it and vice versa.
|
This test failed on windows now: https://github.com/JuliaMusic/MusicManipulations.jl/actions/runs/5146154049/jobs/9264791892?pr=65#step:7:172 and before that it failed on MacOS. Something weird is going on here... |
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #65 +/- ##
==========================================
+ Coverage 51.21% 51.78% +0.56%
==========================================
Files 10 12 +2
Lines 410 477 +67
==========================================
+ Hits 210 247 +37
- Misses 200 230 +30
|
emmm... The only change in |
|
Commit nothing try to re-run ci test, and all passed. |
Done! If you have more time please help me review this issue, Thank you very much. |
Related issue