Skip to content

Conversation

@malcolmsailor
Copy link

@malcolmsailor malcolmsailor commented Dec 10, 2025

This patch fixes the issue described in #263 .

The change is to add 1 to max_tick when only_notes_onsets is True. Then, we calculate the bars in a way that is inclusive of the last onset, rather than exclusive of it. This causes the test case in the linked issue to pass.

If you'd like to add the test case to the tests folder I can do that as well.


📚 Documentation preview 📚: https://miditok--264.org.readthedocs.build/en/264/

@Natooz Natooz added the bug Something isn't working label Dec 10, 2025
@Natooz
Copy link
Owner

Natooz commented Dec 10, 2025

Nice catch, thank you for reporting and bringing the fix!
The comment are clear enough, thank you!
Waiting for the tests to end and it will be ready to be merged (some test cases with abc files are expected to fail due to issues in symusic, non-related to MidiTok)

@Natooz
Copy link
Owner

Natooz commented Dec 15, 2025

Sorry for the late reply. I'll debug the two failing tests (test_pytorch_data_loading.py::test_dataset_midi with pertok) this week-end

@malcolmsailor
Copy link
Author

That's no problem. From your previous comment I understood that any failing tests were likely due to causes other than my PR, but if that's mistaken and this PR is causing tests to newly fail I can debug the failing tests as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants