Allow pre-loaded mido.MidiFile objects to be passed as an argument to PrettyMIDI

[a-pillay]

• Implemented mido_object argument in PrettyMIDI to pass pre-loaded mido.MidiFile objects
• Updated docstring with the aforementioned change
• Implemented a unit test to verify PrettyMIDI object initialization logic

This closes #200 and #201

[craffel]

We actually need to put the new arg at the end of the args list to avoid breaking code that uses positional arguments.

[a-pillay]

@craffel I agree.

I have modified mido_object to be the last argument in PrettyMIDI. Please let me know if anything else needs to be addressed!

[craffel]

Need to change argument order in docstring 😅

[a-pillay]

I was considering setting the mido_object description as the last one in the argument docstring, but proceeded with keeping it immediately after midi_file to highlight how the former will be prioritized.

However, I can set the mido_object description as the last one (in sync with the argument order) if you could confirm that it sounds good. I find both options equally logical.

[craffel]

Docstring argument lists must match the order of the arguments.

[a-pillay]

Sounds good! I have updated the docstrings as suggested. Thanks for putting up with my repeated requests for clarifications!

[craffel]

Can you raise a ValueError if both a filename and a mido object are provided? This should not work silently because the correct behavior is ambiguous. The logic would look like

if mido_object is not None and midi_file is not None:
   raise ValueError("A midi_file and midi_object cannot both be provided.")

and then the rest of the logic can proceed assuming that only one of them has been provided (e.g. you don't need an elif).

[a-pillay]

Done! Now, a ValueError will be raised if both mido_object and midi_file arguments are provided at the same time. Also, updated the concerned docstring and unit test to reflect the same. I have also added tests for verifying these ValueErrors but since the existing tests don't use a toolkit like unittest, I have implemented them in an alternate way.

[craffel]

This conditional is no longer necessary.

[a-pillay]

We need that conditional to handle cases when both the midi_file and mido_object is None; in this case, the logic part of the else block on line 135 will get executed (same as existing logic).

Having this line ensures we only need to implement the logic from line 94 to line 134 once to handle both midi_file and mido_object .

[craffel]

Ah, I missed the preexisting logic. Thanks.

[craffel]

Ah, I missed the preexisting logic. Thanks.

[craffel]

Thanks!