Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#13475 closed Patch - Feature (fixed)

Fix Python Bindings, add python3 compatibility

Reported by: rcrdnalor Owned by: Bill Meek
Priority: minor Milestone: 31.0
Component: Bindings - Python Version: Master Head
Severity: medium Keywords: Python
Cc: Ticket locked: no

Description

Pull request #182 wants to add fixes for the MythTV's python bindings:

This provides compatibility to python3 as well.

See: https://github.com/MythTV/mythtv/pull/182

Note: The announced changes are based on tests from repository at

https://github.com/rcrdnalor/MythTV-Python-Tests

and provide fixes to the bindings and compatibility to python3 as well.

This does not include files from:

  • services_api
  • ttvdb
  • mythnetvision
  • mythmusic

Thanks to Mark providing initial porting to python3.

Attachments (5)

unittests.py (17.3 KB) - added by Bill Meek 6 years ago.
Roland, my unittests for the services_api, if it helps
0001-Fix-typo-in-in-class-OrdDict-of-altdict.py.patch (1.8 KB) - added by rcrdnalor 6 years ago.
Late patch 0001
0002-Fix-UnicodeEncodeError-in-representation-of-class-Vi.patch (2.0 KB) - added by rcrdnalor 6 years ago.
Late patch 0002
0003-Fix-typo-in-SQL-statement-of-class-Video.patch (1.3 KB) - added by rcrdnalor 6 years ago.
Late patch 0003
0004-Remove-unused-code-from-mythproto.py.patch (933 bytes) - added by rcrdnalor 6 years ago.
Late patch 0004

Download all attachments as: .zip

Change History (19)

comment:1 Changed 6 years ago by rcrdnalor

Please ignore the link to the closed ticket (182).

The pull request is still valid:

https://github.com/MythTV/mythtv/pull/182

comment:2 Changed 6 years ago by Bill Meek

Milestone: needs_triage31.0
Owner: changed from Raymond Wagner to Bill Meek
Status: newaccepted
Type: Patch - Bug FixPatch - Feature

rcrdnalor, thanks! Running some operational tests now. Looks great.

FYI, the services_api module works with v2 and v3 already.

comment:3 Changed 6 years ago by rcrdnalor

I found additional incompatibilities to python3:

See the last patches from pull request 182.

Please update your local test environment to include this patch.

The last patch from pull request 182 fixes #12243 as well and the testcases verified, that #12245 cannot be reproduced anymore.

See file 'test_Dataheap_VideoGrabber_001.py' from my repo

https://github.com/rcrdnalor/MythTV-Python-Tests

Additionally, I got an alarming message today at https://lwn.net/Articles/795546/

In short: Python3 will expose the deprecation warnings about invalid escape sequence to the user and will soon report them as error.

On python 3.6, they are only visible with the '-Wa' switch, e.g. if you run

python -Wa -m compileall -f -q MythTV'

Current MythTV Bindings, even with patches form pull request 182, show some warnings of this kind, especially in regex expressions.

I'll try to look into this during weekend.

comment:4 Changed 6 years ago by rcrdnalor

The latest commits on the pull request 182 fixes the deprecation warnings of python3 and some leftovers/typos from previously commits.

Please give it a try.

Note: Once, this pull request is applied, it will close the following bugs:

  • #13299: Python Bindings fail to calculate date-time object
  • #12365: Update Perl and Python bindings proto version and token, see #12365, comment:5
  • #13320: Python bindings not reconnecting to database after a long pause, but in my commit comment, it states wrongly #11938 instead of #13320.
  • #12243: Python Bindings failing with new inetrefid changes
  • #12245: mythvidexport fails when there are non ascii characters in actor names: I cannot reproduce this bug annymore

The Developer Task #8710 "Port python bindings to Python 3.x" has to be re-considered as well:

All but the "MythNetVison" code under 'mythtv/programs/scripts/internetcontent' is ported to Python[2/3].

Additional note: It would be nice if ticket #13300 will be accepted by the MythTV community, too:

I use this in my user jobs to update the properties of transcoded recordings in the 'recordedfile' table every day.

Changed 6 years ago by Bill Meek

Attachment: unittests.py added

Roland, my unittests for the services_api, if it helps

comment:5 Changed 6 years ago by rcrdnalor

I fixed the last known incompatibility to python3 related to 'metaclasses'.

See pull request 182.

The tests can be found on my repo: https://github.com/rcrdnalor/MythTV-Python-Tests

With the last commit from this pull request, the python bindings located at

'mythtv/bindings/python' should be compatible to python3, too.

Please update your test environment.

comment:6 Changed 6 years ago by hobbes1069

Thanks for the effort guys!

Considering I can no longer build MythTV for Fedora on RPM Fusion for releases 31 and 32 (Rawhide), do you think this pull request is safe enough to apply?

And will the pull request likely apply to fixes/30 without too much trouble?

comment:7 Changed 6 years ago by hobbes1069

FYI I found a bug in configure related to Python 3.

# Check for python dependencies
if enabled bindings_python; then
    is_python3                 && python=python2
    check_python               || disable_bindings_python "Python 2.6"
    check_py_lib MySQLdb       || disable_bindings_python "MySQLdb"
    check_py_lib lxml          || disable_bindings_python "lxml"
    check_py_lib urlgrabber    || disable_bindings_python "urlgrabber"
fi

The conditional on the is_python3 line is backwards. If "is_python3" checks positive it exits with code 0 and code 1 if it fails which means the 2nd part get's executed immediately reassigning python3 to python2.

comment:8 Changed 6 years ago by Bill Meek

Hi, thanks for testing. See https://www.mythtv.org/wiki/Python3_Conversion please. You can also do a 'normal' build and then cd to the python bindings and do the make there with PYTHON=python3 there.

What you found appears to be old design intent. If someone chose python3 at ./configure time, it was reset to python2. It will be changed. Also, the dependency on urlgrabber will shut off the bindings.

comment:9 Changed 6 years ago by hobbes1069

I'll take a look at the hints, but I'm really only interested in getting v30 to build for Fedora again. BTW, the pull request did apply cleanly to fixes/30.

comment:10 Changed 6 years ago by hobbes1069

I was able to beat the package into submission and have a package built for Fedora Rawhide (f32) and f31 building. I'll let you know if I get any bug reports related to Python 3.

FYI, I ran 2to3 only running the urllib fix.

comment:11 Changed 6 years ago by rcrdnalor

I updated the pull request 182 with a fix for #13300 an a patch to silence a warning of python3 on exit() on some occasions.

Tests have been added to my testing repository at

https://github.com/rcrdnalor/MythTV-Python-Tests

Please give it a try and update your test installations. See

https://www.mythtv.org/wiki/Python3_Conversion

Note: this pull request now closes the following tickets:

  • #12243 Python Bindings failing with new inetrefid changes
  • #12245 mythvidexport fails when there are non ascii characters in actor names.
  • #13320 Python bindings not reconnecting to database after a long pause.
  • #12365 Patch: Update Perl and Python bindings proto version and token
  • #13299 Python Bindings fail to calculate date-time object
  • #13300 Python Bindings enhancemts according to latest MythTV protocol (91)

comment:12 Changed 6 years ago by rcrdnalor

Late fixes on python3 compatibility of the Python Bindings:

With the attached patches, one gets a reasonable code coverage for the python's MySQLdb statements.

This can be used to port the sql driver 'MySQLdb' to 'pymysql' or to 'python-mysql-connector', as suggested by #13502.

See my repo for test results on

https://github.com/rcrdnalor/MythTV-Python-Tests

and especially the test-set defined by

$ run_all_tests_separated.sh --minimal

Changed 6 years ago by rcrdnalor

Late patch 0001

Changed 6 years ago by rcrdnalor

Late patch 0002

Changed 6 years ago by rcrdnalor

Late patch 0003

Changed 6 years ago by rcrdnalor

Late patch 0004

comment:13 Changed 6 years ago by Roland Ernst <rcrernst@…>

Resolution: fixed
Status: acceptedclosed

In bafc80d9ff/mythtv:

Error: Processor CommitTicketReference failed
GIT backend not available

comment:14 Changed 5 years ago by Roland Ernst <rcrernst@…>

In c2ff157ca/mythtv:

Error: Processor CommitTicketReference failed
GIT backend not available
Note: See TracTickets for help on using tickets.