Python 3 support, continued #130

mgedmin · 2017-10-10T18:56:31Z

This is a continuation of @florianfesti's excellent PR #35. It fixes some bugs and made the test suite pass on Pythons 2 and 3 on my machine.

Those properties are within the classes and don't need space in the instances. Py3k complaints about them as the properies shadow the slots.

This gets rid of all those annoying ord() calls Use next() instead of .next() Use b"" where comparing with the raw data

Minor reformat

Ask for read() and write() attribute to avoid check for unicode which does not exist in Python3.

May need some love

The tests now pass (again) on Python 2.7.

I'm getting that error on Python 3 and I don't quite understand how the sequencer module works here (src/sequencer.py doesn't define a Sequencer class!).

(I don't know why Event comparison ignores everything except tick, seems like a bad idea TBH, but it's what master does so *shrug*) Note that these methods appear to be untested.

This is suboptimal on Python 2, but I don't expect these dicts to have many values, so it should be OK.

mgedmin · 2017-10-10T19:45:09Z

I believe I've figured out how to raise exceptions in C code that is wrapped by SWIG. Now the tests fail because you cannot open /dev/snd/seq in the Travis environment.

What's a bit more annoying is that tests fail for me locally with

======================================================================
ERROR: test_loopback_sequencer (tests.TestSequencerALSA)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mg/src/python-midi/tests/tests.py", line 76, in test_loopback_sequencer
    recv_event = rseq.event_read()
  File "/home/mg/src/python-midi/.tox/py27/local/lib/python2.7/site-packages/midi/sequencer/sequencer.py", line 278, in event_read
    ev = S.event_input(self.client)
IOError: Resource temporarily unavailable

I don't know how the ALSA loopback sequencer is supposed to work, so I'm not sure how to debug this.

coveralls · 2017-10-10T19:54:18Z

Coverage increased (+0.1%) to 68.201% when pulling d0d01c4 on mgedmin:python3 into 4b7a229 on vishnubob:master.

When you're reading from a file descriptor in non-blocking mode and there's nothing to read, you get an EAGAIN error. We don't want to see those as exceptions; we want to see a None value returned by event_input().

coveralls · 2017-10-10T20:01:06Z

Coverage increased (+0.2%) to 68.212% when pulling d0d01c4 on mgedmin:python3 into 4b7a229 on vishnubob:master.

mgedmin · 2017-10-10T20:01:16Z

And I've figured out that the Resource temporarily unavailable error is normal and should be ignored, and now all three tests pass on my machine!

I think this is ready for merging. I'd appreciate if any other users cared to test this branch on either Python 2 or 3.

coveralls · 2017-10-10T20:02:21Z

Coverage increased (+0.1%) to 68.201% when pulling fbde410 on mgedmin:python3 into 4b7a229 on vishnubob:master.

mgedmin · 2017-10-10T20:05:19Z

src/sequencer_alsa/sequencer_alsa.i

+    if (err == -EAGAIN)
+    {
+        Py_INCREF(Py_None);
+        return Py_None;


This produces a compilation warning:

src/sequencer_alsa/sequencer_alsa_wrap.c:3039:9: warning: return from incompatible pointer type [enabled by default] return Py_None; ^

And while it seems to work fine on my machine, TBH I'm not sure it's safe to pass Py_None through SWIG's SWIG_NewPointerObj() wrapper like this.

coveralls · 2017-10-10T20:12:08Z

Coverage increased (+0.1%) to 68.201% when pulling fec1e8f on mgedmin:python3 into 4b7a229 on vishnubob:master.

rosstex · 2017-11-05T20:26:17Z

Godspeed!

florianfesti

Trivial Change. Fixes dependency on future module.

florianfesti · 2017-11-12T14:07:58Z

Sorry, I thought I could review single patches. So my review above is premature. Will look through the other patches now.

florianfesti · 2017-11-12T14:57:49Z

After looking through all patches: This looks really good. I strongly support merging this PR.

One might argue that it might be even better if the two or three patches that fix previous ones would be rebased and squashed but this is rather cosmetic. Candidates might be:

Include own copy of with_metaclass()
Python 2 iterator protocol compatibility
which could be merged with my patches that create the issues in the first place and
Fix compiler warning about incompatible return types

But the patch set is good enough as is to be just merged IMHO.

Thank you very much for the effort!

cclauss · 2019-09-22T17:00:04Z

100 days until Python 2 end of life.

See: - https://github.com/mgedmin/python-midi - vishnubob#130

* Python3 port by Marius Gedminas See: - https://github.com/mgedmin/python-midi - vishnubob#130 * Re-applied adaptations for samplerbox * Add buffered writing of 1-track midi files (#2)

florianfesti and others added 25 commits January 12, 2017 11:48

Py3k compat: make imports of sub modules relativ

a1e5d63

Py3k compat: Remove errous __slot__ definitions

a4ef006

Those properties are within the classes and don't need space in the instances. Py3k complaints about them as the properies shadow the slots.

Py3k compat: Use with_metaclass for compatible meta classing

2db4b81

Py3k compat: Instantiate Exceptions and use repr() instead of backticks

1fc0bd6

Py3k compat: make data read a bytearray in both Python2 and Python3

dcd62bc

This gets rid of all those annoying ord() calls Use next() instead of .next() Use b"" where comparing with the raw data

Py3k compat: wrap ranges with list()

e78bd9a

Minor reformat

Py3k compat: Change Filewriter to use bytes and bytearrays

b22f61f

Ask for read() and write() attribute to avoid check for unicode which does not exist in Python3.

Py3k compat: Use rich comparisons

5e85385

May need some love

Py3k compat: Use print_function from __future__

7ae6ec3

Py3k compat: replace xrange by range

6d63385

Include own copy of with_metaclass()

5db02c1

Add a tox.ini

a346920

Work around issue vishnubob#129 so I can run tests

6372115

Fix byte ordering issues in write_varlen()

7834abc

Fix MIDI writing

6e38251

The tests now pass (again) on Python 2.7.

Work around AttributeError: 'module' object has no attribute 'Sequencer'

8888c95

I'm getting that error on Python 3 and I don't quite understand how the sequencer module works here (src/sequencer.py doesn't define a Sequencer class!).

Fix tests under Python 3

6e1c2a7

Enable Python 3 on Travis

7aa5e7b

Make comparison logic match git master

db46eb1

(I don't know why Event comparison ignores everything except tick, seems like a bad idea TBH, but it's what master does so *shrug*) Note that these methods appear to be untested.

Python 2 iterator protocol compatibility

4ba6d3c

Use relative imports for the sequencer as well

2f7f9db

Python 3: instantiate exceptions

3f2d806

Python 3: there's no itervalues() any more

0597aff

This is suboptimal on Python 2, but I don't expect these dicts to have many values, so it should be OK.

Figure out SWIG exception raising

12d5b40

Fix SWIG exception handling for all functions

c2bf158

Check whether /dev/snd/seq is readable/writable

d0d01c4

Make event_input ignore EAGAIN errors

fbde410

When you're reading from a file descriptor in non-blocking mode and there's nothing to read, you get an EAGAIN error. We don't want to see those as exceptions; we want to see a None value returned by event_input().

mgedmin commented Oct 10, 2017

View reviewed changes

Fix compiler warning about incompatible return types

fec1e8f

mgedmin mentioned this pull request Oct 11, 2017

Python 3 compatibility #109

Open

florianfesti approved these changes Nov 12, 2017

View reviewed changes

aamini mentioned this pull request May 16, 2018

Python-midi not available for Python 3.6.5 aamini/introtodeeplearning#17

Closed

This was referenced Jun 5, 2018

Modernize Python 2 code to get ready for Python 3 #142

Closed

Add a link to the python3 branch #118

Open

Python 3 #35

Open

LESSSE approved these changes Aug 4, 2019

View reviewed changes

hansehv added a commit to hansehv/python-midi that referenced this pull request Jul 5, 2020

Python3 port by Marius Gedminas

b39e5b1

See: - https://github.com/mgedmin/python-midi - vishnubob#130

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python 3 support, continued #130

Python 3 support, continued #130

mgedmin commented Oct 10, 2017 •

edited

Loading

mgedmin commented Oct 10, 2017

coveralls commented Oct 10, 2017 •

edited

Loading

coveralls commented Oct 10, 2017 •

edited

Loading

mgedmin commented Oct 10, 2017

coveralls commented Oct 10, 2017 •

edited

Loading

mgedmin Oct 10, 2017

coveralls commented Oct 10, 2017 •

edited

Loading

rosstex commented Nov 5, 2017

florianfesti left a comment

florianfesti commented Nov 12, 2017

florianfesti commented Nov 12, 2017

cclauss commented Sep 22, 2019

Python 3 support, continued #130

Are you sure you want to change the base?

Python 3 support, continued #130

Conversation

mgedmin commented Oct 10, 2017 • edited Loading

mgedmin commented Oct 10, 2017

coveralls commented Oct 10, 2017 • edited Loading

coveralls commented Oct 10, 2017 • edited Loading

mgedmin commented Oct 10, 2017

coveralls commented Oct 10, 2017 • edited Loading

mgedmin Oct 10, 2017

Choose a reason for hiding this comment

coveralls commented Oct 10, 2017 • edited Loading

rosstex commented Nov 5, 2017

florianfesti left a comment

Choose a reason for hiding this comment

florianfesti commented Nov 12, 2017

florianfesti commented Nov 12, 2017

cclauss commented Sep 22, 2019

mgedmin commented Oct 10, 2017 •

edited

Loading

coveralls commented Oct 10, 2017 •

edited

Loading

coveralls commented Oct 10, 2017 •

edited

Loading

coveralls commented Oct 10, 2017 •

edited

Loading

coveralls commented Oct 10, 2017 •

edited

Loading