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

Wrong code comments about relative and absolute time? #526

Open
page200 opened this issue Aug 1, 2023 · 1 comment · May be fixed by #531
Open

Wrong code comments about relative and absolute time? #526

page200 opened this issue Aug 1, 2023 · 1 comment · May be fixed by #531
Assignees
Labels
bug feature:midifile Standard MIDI File (SMF) implementation

Comments

@page200
Copy link

page200 commented Aug 1, 2023

Is my understanding correct that "absolute time" can be interpreted as the time that has passed since the beginning of a song, whereas "relative time" can be interpreted as the time that has passed since the previous MIDI message?

If so, then tick2second() and second2tick() are capable of converting absolute time in ticks or seconds to absolute time in seconds or ticks, as well as converting relative time in ticks or seconds to relative time in seconds or ticks. And they are often used for the latter (converting relative time).

Therefore, the four occurrences of "absolute" should be removed from their docstrings.

And then this comment is wrong: relative (not absolute) time in ticks is being converted to relative time in seconds there.

By the way, why the singular "tick", "second" rather than "ticks", "seconds" in the name of the functions tick2second() and second2tick() and their input argument tick?

@page200 page200 added the bug label Aug 1, 2023
@rileyjshaw
Copy link

Thanks @page200 for trying to clean up some of the time language and inconsistencies (also #527). It’s surely one of the more difficult aspects of this library.

@rdoursenaud rdoursenaud added the feature:midifile Standard MIDI File (SMF) implementation label Aug 4, 2023
@rdoursenaud rdoursenaud self-assigned this Aug 4, 2023
rdoursenaud added a commit to rdoursenaud/mido that referenced this issue Aug 4, 2023
Deprecates tick2second in favor of ticks2seconds and second2tick in
favor of seconds2ticks.
Returns fractional time is seconds per mido#528
Removes erroneous references to absolute in docstrings and comments.

Fixes mido#526
rdoursenaud added a commit to rdoursenaud/mido that referenced this issue Aug 4, 2023
Deprecates tick2second in favor of ticks2seconds and second2tick in
favor of seconds2ticks.
Returns fractional time is seconds per mido#528
Removes erroneous references to absolute in docstrings and comments.

Fixes mido#526
rdoursenaud added a commit to rdoursenaud/mido that referenced this issue Aug 4, 2023
Deprecates tick2second in favor of ticks2seconds and second2tick in
favor of seconds2ticks.
Returns fractional time is seconds per mido#528
Removes erroneous references to absolute in docstrings and comments.

Fixes mido#526
rdoursenaud added a commit to rdoursenaud/mido that referenced this issue Aug 4, 2023
Deprecates tick2second in favor of ticks2seconds and second2tick in
favor of seconds2ticks.
Returns fractional time is seconds per mido#528
Removes erroneous references to absolute in docstrings and comments.

Fixes mido#526
rdoursenaud added a commit to rdoursenaud/mido that referenced this issue Aug 4, 2023
Deprecates tick2second in favor of ticks2seconds and second2tick in
favor of seconds2ticks.
Returns fractional time is seconds per mido#528
Removes erroneous references to absolute in docstrings and comments.

Fixes mido#526
@rdoursenaud rdoursenaud linked a pull request Aug 4, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug feature:midifile Standard MIDI File (SMF) implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants