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.

Sign up for GitHub

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

Jump to bottom

Python 3 port #411

Merged
merged 8 commits into from
Nov 26, 2019
Merged

Python 3 port #411

merged 8 commits into from
Nov 26, 2019

Conversation

ddevault
Copy link
Contributor

Given the imminent end-of-life for Python 2, I didn't bother making the codebase compatible with both.

I successfully ripped a CD with this patch using Python 3, but I'm not confident that I hit every code path. Hopefully this is useful as a jumping-off point for someone who wants to finish the port.

@welcome
Copy link

welcome bot commented Aug 10, 2019

💖 Thanks for opening your first pull request here! 💖

@MerlijnWajer
Copy link
Collaborator

MerlijnWajer commented Aug 10, 2019 •
edited
Loading

Nice. This change is unrelated to the port though right:

https://github.com/whipper-team/whipper/pull/411/files#diff-d21e88dd3c71bbba68f7560bd5bfc6f0R223

(Otherwise - great work, will try it out over the weekend)

@MerlijnWajer
Copy link
Collaborator

Nevermind, python3 doesn't do Nonetype comparisons, alright, cool.

@JoeLametta JoeLametta changed the title Initial pass on python 3 port WIP: Initial pass on python 3 port Aug 10, 2019
@JoeLametta
Copy link
Collaborator

Thanks for the pull request!
I've just pushed a commit to that branch in order to update the CI configuration (to make it work with Python 3). As Travis CI needs a specific Python version set, I've arbitrarily chosen 3.5.

@Freso Freso added this to the 0.9.0 milestone Aug 10, 2019
@JoeLametta
Copy link
Collaborator

I've fixed some issues but it's still a WIP.

  • Tests status as in d47e893de7573429782cd4d170dc8d7b2eea6b33: FAILED (failures=3, errors=26).
  • Right now (dd0a1863c37481408e4ccf85ce5a4a1cde3f37a4): FAILED (failures=2, errors=9).
Test report 
======================================================================
ERROR: test_report_v1_only (whipper.test.test_common_accurip.TestAccurateRipReport)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/whipper-team/whipper/whipper/test/test_common_accurip.py", line 284, in test_report_v1_only
    print_report(self.result)
  File "/home/travis/build/whipper-team/whipper/whipper/common/accurip.py", line 253, in print_report
    [track.AR[v]['DBConfidence'] for v in ('v1', 'v2')]
TypeError: unorderable types: NoneType() > int()

======================================================================
ERROR: test_report_v2_only (whipper.test.test_common_accurip.TestAccurateRipReport)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/whipper-team/whipper/whipper/test/test_common_accurip.py", line 294, in test_report_v2_only
    print_report(self.result)
  File "/home/travis/build/whipper-team/whipper/whipper/common/accurip.py", line 253, in print_report
    [track.AR[v]['DBConfidence'] for v in ('v1', 'v2')]
TypeError: unorderable types: int() > NoneType()

======================================================================
ERROR: test_incomplete_checksums (whipper.test.test_common_accurip.TestVerifyResult)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/whipper-team/whipper/whipper/test/test_common_accurip.py", line 150, in test_incomplete_checksums
    'v2': [None, 'dd97d2c3'],
  File "/home/travis/build/whipper-team/whipper/whipper/common/accurip.py", line 232, in verify_result
    return _match_responses(tracks, responses)
  File "/home/travis/build/whipper-team/whipper/whipper/common/accurip.py", line 204, in _match_responses
    if r.confidences[i] > track.AR[v]['DBConfidence']:
TypeError: unorderable types: int() > NoneType()

======================================================================
ERROR: test_matches_only_v1_or_v2_responses (whipper.test.test_common_accurip.TestVerifyResult)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/whipper-team/whipper/whipper/test/test_common_accurip.py", line 172, in test_matches_only_v1_or_v2_responses
    self.result, [self.responses[0]], self.checksums
  File "/home/travis/build/whipper-team/whipper/whipper/common/accurip.py", line 232, in verify_result
    return _match_responses(tracks, responses)
  File "/home/travis/build/whipper-team/whipper/whipper/common/accurip.py", line 204, in _match_responses
    if r.confidences[i] > track.AR[v]['DBConfidence']:
TypeError: unorderable types: int() > NoneType()

======================================================================
ERROR: test_passes_with_htoa (whipper.test.test_common_accurip.TestVerifyResult)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/whipper-team/whipper/whipper/test/test_common_accurip.py", line 188, in test_passes_with_htoa
    verify_result(self.result, self.responses, self.checksums),
  File "/home/travis/build/whipper-team/whipper/whipper/common/accurip.py", line 232, in verify_result
    return _match_responses(tracks, responses)
  File "/home/travis/build/whipper-team/whipper/whipper/common/accurip.py", line 204, in _match_responses
    if r.confidences[i] > track.AR[v]['DBConfidence']:
TypeError: unorderable types: int() > NoneType()

======================================================================
ERROR: test_stores_accuraterip_results_on_result (whipper.test.test_common_accurip.TestVerifyResult)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/whipper-team/whipper/whipper/test/test_common_accurip.py", line 194, in test_stores_accuraterip_results_on_result
    verify_result(self.result, self.responses, self.checksums),
  File "/home/travis/build/whipper-team/whipper/whipper/common/accurip.py", line 232, in verify_result
    return _match_responses(tracks, responses)
  File "/home/travis/build/whipper-team/whipper/whipper/common/accurip.py", line 204, in _match_responses
    if r.confidences[i] > track.AR[v]['DBConfidence']:
TypeError: unorderable types: int() > NoneType()

======================================================================
ERROR: testLength (whipper.test.test_program_soxi.AudioLengthTestCase)
testLength
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/defer.py", line 151, in maybeDeferred
    result = f(*args, **kw)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/utils.py", line 221, in runWithWarningsSuppressed
    reraise(exc_info[1], exc_info[2])
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/python/compat.py", line 464, in reraise
    raise exception.with_traceback(traceback)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/utils.py", line 217, in runWithWarningsSuppressed
    result = f(*a, **kw)
  File "/home/travis/build/whipper-team/whipper/whipper/test/test_program_soxi.py", line 21, in testLength
    runner.run(t, verbose=False)
  File "/home/travis/build/whipper-team/whipper/whipper/extern/task/task.py", line 499, in run
    raise TaskException(task.exception, message=msg)
whipper.extern.task.task.TaskException: (TypeError('sequence item 0: expected str instance, bytes found',), 'exception TypeError at /home/travis/build/whipper-team/whipper/whipper/program/soxi.py:50: done(): sequence item 0: expected str instance, bytes found\nTraceback (most recent call last):\n  File "/home/travis/build/whipper-team/whipper/whipper/common/task.py", line 89, in _read\n    self._done()\n  File "/home/travis/build/whipper-team/whipper/whipper/common/task.py", line 109, in _done\n    self.done()\n  File "/home/travis/build/whipper-team/whipper/whipper/program/soxi.py", line 50, in done\n    self.length = int("".join(self._output))\nTypeError: sequence item 0: expected str instance, bytes found\n')

======================================================================
ERROR: testDoubleQuote (whipper.test.test_program_soxi.NormalAudioLengthPathTestCase)
testDoubleQuote
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/defer.py", line 151, in maybeDeferred
    result = f(*args, **kw)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/utils.py", line 221, in runWithWarningsSuppressed
    reraise(exc_info[1], exc_info[2])
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/python/compat.py", line 464, in reraise
    raise exception.with_traceback(traceback)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/utils.py", line 217, in runWithWarningsSuppressed
    result = f(*a, **kw)
  File "/home/travis/build/whipper-team/whipper/whipper/test/test_program_soxi.py", line 47, in testDoubleQuote
    self._testSuffix('whipper.test.12" edit.flac')
  File "/home/travis/build/whipper-team/whipper/whipper/test/test_program_soxi.py", line 34, in _testSuffix
    runner.run(t, verbose=False)
  File "/home/travis/build/whipper-team/whipper/whipper/extern/task/task.py", line 499, in run
    raise TaskException(task.exception, message=msg)
whipper.extern.task.task.TaskException: (TypeError('sequence item 0: expected str instance, bytes found',), 'exception TypeError at /home/travis/build/whipper-team/whipper/whipper/program/soxi.py:50: done(): sequence item 0: expected str instance, bytes found\nTraceback (most recent call last):\n  File "/home/travis/build/whipper-team/whipper/whipper/common/task.py", line 89, in _read\n    self._done()\n  File "/home/travis/build/whipper-team/whipper/whipper/common/task.py", line 109, in _done\n    self.done()\n  File "/home/travis/build/whipper-team/whipper/whipper/program/soxi.py", line 50, in done\n    self.length = int("".join(self._output))\nTypeError: sequence item 0: expected str instance, bytes found\n')

======================================================================
ERROR: testSingleQuote (whipper.test.test_program_soxi.NormalAudioLengthPathTestCase)
testSingleQuote
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/defer.py", line 151, in maybeDeferred
    result = f(*args, **kw)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/utils.py", line 221, in runWithWarningsSuppressed
    reraise(exc_info[1], exc_info[2])
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/python/compat.py", line 464, in reraise
    raise exception.with_traceback(traceback)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/utils.py", line 217, in runWithWarningsSuppressed
    result = f(*a, **kw)
  File "/home/travis/build/whipper-team/whipper/whipper/test/test_program_soxi.py", line 42, in testSingleQuote
    self._testSuffix("whipper.test.Guns 'N Roses.flac")
  File "/home/travis/build/whipper-team/whipper/whipper/test/test_program_soxi.py", line 34, in _testSuffix
    runner.run(t, verbose=False)
  File "/home/travis/build/whipper-team/whipper/whipper/extern/task/task.py", line 499, in run
    raise TaskException(task.exception, message=msg)
whipper.extern.task.task.TaskException: (TypeError('sequence item 0: expected str instance, bytes found',), 'exception TypeError at /home/travis/build/whipper-team/whipper/whipper/program/soxi.py:50: done(): sequence item 0: expected str instance, bytes found\nTraceback (most recent call last):\n  File "/home/travis/build/whipper-team/whipper/whipper/common/task.py", line 89, in _read\n    self._done()\n  File "/home/travis/build/whipper-team/whipper/whipper/common/task.py", line 109, in _done\n    self.done()\n  File "/home/travis/build/whipper-team/whipper/whipper/program/soxi.py", line 50, in done\n    self.length = int("".join(self._output))\nTypeError: sequence item 0: expected str instance, bytes found\n')

======================================================================
FAIL: testGetResult (whipper.test.test_common_cache.ResultCacheTestCase)
testGetResult
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/defer.py", line 151, in maybeDeferred
    result = f(*args, **kw)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/utils.py", line 221, in runWithWarningsSuppressed
    reraise(exc_info[1], exc_info[2])
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/python/compat.py", line 464, in reraise
    raise exception.with_traceback(traceback)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/utils.py", line 217, in runWithWarningsSuppressed
    result = f(*a, **kw)
  File "/home/travis/build/whipper-team/whipper/whipper/test/test_common_cache.py", line 19, in testGetResult
    self.assertEqual(result.object.title, "The Writing's on the Wall")
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/trial/_synctest.py", line 432, in assertEqual
    super(_Assertions, self).assertEqual(first, second, msg)
  File "/opt/python/3.5.6/lib/python3.5/unittest/case.py", line 829, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/opt/python/3.5.6/lib/python3.5/unittest/case.py", line 822, in _baseAssertEqual
    raise self.failureException(msg)
twisted.trial.unittest.FailTest: None != "The Writing's on the Wall"

======================================================================
FAIL: testDuration (whipper.test.test_image_toc.CapitalMergeTestCase)
testDuration
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/defer.py", line 151, in maybeDeferred
    result = f(*args, **kw)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/utils.py", line 221, in runWithWarningsSuppressed
    reraise(exc_info[1], exc_info[2])
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/python/compat.py", line 464, in reraise
    raise exception.with_traceback(traceback)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/utils.py", line 217, in runWithWarningsSuppressed
    result = f(*a, **kw)
  File "/home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py", line 271, in testDuration
    self.assertEqual(self.table.getFrameLength(), 173530)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/trial/_synctest.py", line 432, in assertEqual
    super(_Assertions, self).assertEqual(first, second, msg)
  File "/opt/python/3.5.6/lib/python3.5/unittest/case.py", line 829, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/opt/python/3.5.6/lib/python3.5/unittest/case.py", line 822, in _baseAssertEqual
    raise self.failureException(msg)
twisted.trial.unittest.FailTest: 184930 != 173530

----------------------------------------------------------------------
Ran 109 tests in 6.427s

FAILED (failures=2, errors=9)

There also many ResourceWarning: unclosed file warnings:

Warnings 
...EE./home/travis/build/whipper-team/whipper/whipper/test/test_common_accurip.py:25: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/dBAR-002-0000f21c-00027ef8-05021002.bin'>
  open(join(dirname(__file__), cls.path[6:]), "rb").read()
/home/travis/build/whipper-team/whipper/whipper/common/accurip.py:158: ResourceWarning: unclosed file <_io.BufferedWriter name='/tmp/tmpu9j59kjgwhipper_accurip_cache_test/c/1/2/dBAR-002-0000f21c-00027ef8-05021002.bin'>
  open(path, 'wb').write(raw_entry)
./home/travis/build/whipper-team/whipper/whipper/common/accurip.py:158: ResourceWarning: unclosed file <_io.BufferedWriter name='/tmp/tmpik93xsymwhipper_accurip_cache_test/c/1/2/dBAR-002-0000f21c-00027ef8-05021002.bin'>
  open(path, 'wb').write(raw_entry)
./home/travis/build/whipper-team/whipper/whipper/common/accurip.py:171: ResourceWarning: unclosed file <_io.BufferedReader name='/tmp/tmpdkn2z_n9whipper_accurip_cache_test/4/8/2/dBAR-011-0010e284-009228a3-9809ff0b.bin'>
  raw_entry = open(cached_path, 'rb').read()
./home/travis/build/whipper-team/whipper/whipper/test/test_common_accurip.py:104: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/dBAR-002-0000f21c-00027ef8-05021002.bin'>
  open(join(dirname(__file__), path[6:]), "rb").read()
...EEEE..F/home/travis/build/whipper-team/whipper/whipper/common/cache.py:55: ResourceWarning: unclosed file <_io.TextIOWrapper name='/home/travis/build/whipper-team/whipper/whipper/test/cache/result/fe105a11.pickle' mode='r' encoding='UTF-8'>
  self._unpickle(default)
............../home/travis/build/whipper-team/whipper/whipper/test/test_common_renamer.py:69: ResourceWarning: unclosed file <_io.TextIOWrapper name='/tmp/tmpcji923yn.whipper.renamer.file' mode='r' encoding='UTF-8'>
  output = open(self._destination).read()
./home/travis/build/whipper-team/whipper/whipper/test/test_common_renamer.py:77: ResourceWarning: unclosed file <_io.TextIOWrapper name='/tmp/tmp254kybzk.whipper.renamer.file' mode='r' encoding='UTF-8'>
  output = open(self._destination).read()
../home/travis/build/whipper-team/whipper/whipper/test/test_common_renamer.py:28: ResourceWarning: unclosed file <_io.TextIOWrapper name='/tmp/tmpa8x0m1vy.whipper.renamer.infile' mode='r' encoding='UTF-8'>
  output = open(self._path).read()
./home/travis/build/whipper-team/whipper/whipper/test/test_common_renamer.py:37: ResourceWarning: unclosed file <_io.TextIOWrapper name='/tmp/tmpa6khud25.whipper.renamer.infile' mode='r' encoding='UTF-8'>
  output = open(self._path).read()
..INFO:whipper.image.cue:parsing .cue file '/home/travis/build/whipper-team/whipper/whipper/test/kanye.cue'
/home/travis/build/whipper-team/whipper/whipper/test/test_image_cue.py:52: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/kanye.cue'>
  self.cue.parse()
.INFO:whipper.image.cue:parsing .cue file '/home/travis/build/whipper-team/whipper/whipper/test/kings-separate.cue'
/home/travis/build/whipper-team/whipper/whipper/test/test_image_cue.py:36: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/kings-separate.cue'>
  self.cue.parse()
.INFO:whipper.image.cue:parsing .cue file '/home/travis/build/whipper-team/whipper/whipper/test/kings-single.cue'
/home/travis/build/whipper-team/whipper/whipper/test/test_image_cue.py:20: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/kings-single.cue'>
  self.cue.parse()
........../home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:113: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/bloc.toc'>
  self.toc.parse()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:113: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/bloc.toc'>
  self.toc.parse()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:113: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/bloc.toc'>
  self.toc.parse()
/home/travis/build/whipper-team/whipper/whipper/test/common.py:73: ResourceWarning: unclosed file <_io.TextIOWrapper name='/home/travis/build/whipper-team/whipper/whipper/test/bloc.cue' mode='r' encoding='UTF-8'>
  ret = open(cuefile).read()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:113: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/bloc.toc'>
  self.toc.parse()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:113: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/bloc.toc'>
  self.toc.parse()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:178: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/breeders.toc'>
  self.toc.parse()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:178: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/breeders.toc'>
  self.toc.parse()
/home/travis/build/whipper-team/whipper/whipper/test/common.py:73: ResourceWarning: unclosed file <_io.TextIOWrapper name='/home/travis/build/whipper-team/whipper/whipper/test/breeders.cue' mode='r' encoding='UTF-8'>
  ret = open(cuefile).read()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:241: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/capital.1.toc'>
  self.toc1.parse()
/home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:247: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/capital.2.toc'>
  self.toc2.parse()
F/home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:241: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/capital.1.toc'>
  self.toc1.parse()
/home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:247: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/capital.2.toc'>
  self.toc2.parse()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:241: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/capital.1.toc'>
  self.toc1.parse()
/home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:247: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/capital.2.toc'>
  self.toc2.parse()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:19: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/cure.toc'>
  self.toc.parse()
/home/travis/build/whipper-team/whipper/whipper/test/common.py:73: ResourceWarning: unclosed file <_io.TextIOWrapper name='/home/travis/build/whipper-team/whipper/whipper/test/cure.cue' mode='r' encoding='UTF-8'>
  ret = open(cuefile).read()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:19: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/cure.toc'>
  self.toc.parse()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:19: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/cure.toc'>
  self.toc.parse()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:19: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/cure.toc'>
  self.toc.parse()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:19: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/cure.toc'>
  self.toc.parse()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:329: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/gentlemen.fast.toc'>
  self.toc.parse()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:205: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/ladyhawke.toc'>
  self.toc.parse()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:205: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/ladyhawke.toc'>
  self.toc.parse()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:205: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/ladyhawke.toc'>
  self.toc.parse()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:205: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/ladyhawke.toc'>
  self.toc.parse()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:205: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/ladyhawke.toc'>
  self.toc.parse()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:346: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/strokes-someday.toc'>
  self.toc.parse()
/home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:367: ResourceWarning: unclosed file <_io.TextIOWrapper name='/home/travis/build/whipper-team/whipper/whipper/test/strokes-someday.eac.cue' mode='r' encoding='UTF-8'>
  'strokes-someday.eac.cue')).read())
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:405: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/surferrosa.toc'>
  self.toc.parse()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:316: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/totbl.fast.toc'>
  self.toc.parse()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:288: ResourceWarning: unclosed file <_io.BufferedReader name='/tmp/tmpofw98h_2JoséGonzález.toc'>
  self.toc.parse()
./home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py:288: ResourceWarning: unclosed file <_io.BufferedReader name='/tmp/tmpzep3urfnJoséGonzález.toc'>
  self.toc.parse()
...../home/travis/build/whipper-team/whipper/whipper/program/cdparanoia.py:563: ResourceWarning: unclosed file <_io.BufferedReader name=14>
  return getter.get()
/home/travis/build/whipper-team/whipper/whipper/program/cdparanoia.py:563: ResourceWarning: unclosed file <_io.BufferedWriter name=13>
  return getter.get()
....E/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/python/failure.py:766: ResourceWarning: unclosed file <_io.FileIO name=15 mode='wb' closefd=True>
  return [(name, reflect.safe_repr(obj)) for (name, obj) in varsDictItems]
E/home/travis/build/whipper-team/whipper/whipper/test/test_program_soxi.py:30: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/track.flac'>
  temptrack.write(open(base_track_file, "rb").read())
E/home/travis/build/whipper-team/whipper/whipper/test/test_program_soxi.py:30: ResourceWarning: unclosed file <_io.BufferedReader name='/home/travis/build/whipper-team/whipper/whipper/test/track.flac'>
  temptrack.write(open(base_track_file, "rb").read())

@JoeLametta
Copy link
Collaborator

JoeLametta commented Aug 12, 2019 •
edited
Loading

Test status as in 0b8d4d3249c47650ea4cba08a0b082c8eb6b36ba: FAILED (failures=2, errors=6).

I've tried to fix the warnings. Only one ResourceWarning is left:

/opt/python/3.5.6/lib/python3.5/contextlib.py:159: ResourceWarning: unclosed file <_io.FileIO name=15 mode='wb' closefd=True>
  self.thing = thing
/opt/python/3.5.6/lib/python3.5/contextlib.py:159: ResourceWarning: unclosed file <_io.FileIO name=16 mode='wb' closefd=True>
  self.thing = thing
/opt/python/3.5.6/lib/python3.5/contextlib.py:159: ResourceWarning: unclosed file <_io.FileIO name=17 mode='wb' closefd=True>
  self.thing = thing

I think it's in whipper/extern/asyncsub.py (probably in the _recv method).
Before adding the context manager in VersionGetter (whipper/common/common.py) the warnings were these:

/home/travis/build/whipper-team/whipper/whipper/program/cdparanoia.py:563: ResourceWarning: unclosed file <_io.BufferedReader name=14>
  return getter.get()
/home/travis/build/whipper-team/whipper/whipper/program/cdparanoia.py:563: ResourceWarning: unclosed file <_io.BufferedWriter name=13>
  return getter.get()
/home/travis/virtualenv/python3.5.6/lib/python3.5/os.py:690: ResourceWarning: unclosed file <_io.FileIO name=15 mode='wb' closefd=True>
  path_listb = env[b'PATH']
/home/travis/virtualenv/python3.5.6/lib/python3.5/os.py:690: ResourceWarning: unclosed file <_io.FileIO name=16 mode='wb' closefd=True>
  path_listb = env[b'PATH']

@ArchangeGabriel
Copy link

The 6 remaining errors are NoneType comparison, which as stated before by @MerlijnWajer Python 3 does not do.

@JoeLametta
Copy link
Collaborator

Only one test failure left:

FAIL: testDuration (whipper.test.test_image_toc.CapitalMergeTestCase)
testDuration
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/defer.py", line 151, in maybeDeferred
    result = f(*args, **kw)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/utils.py", line 221, in runWithWarningsSuppressed
    reraise(exc_info[1], exc_info[2])
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/python/compat.py", line 464, in reraise
    raise exception.with_traceback(traceback)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/utils.py", line 217, in runWithWarningsSuppressed
    result = f(*a, **kw)
  File "/home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py", line 271, in testDuration
    self.assertEqual(self.table.getFrameLength(), 173530)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/trial/_synctest.py", line 432, in assertEqual
    super(_Assertions, self).assertEqual(first, second, msg)
  File "/opt/python/3.5.6/lib/python3.5/unittest/case.py", line 829, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/opt/python/3.5.6/lib/python3.5/unittest/case.py", line 822, in _baseAssertEqual
    raise self.failureException(msg)
twisted.trial.unittest.FailTest: 184930 != 173530

It may be caused by the different way Python 3 handles division.

BTW: the warning is still there...

@JoeLametta
Copy link
Collaborator

pylint 2.3.1 porting mode output as in 505ea10:

$ pylint --py3k whipper/
************* Module whipper
whipper/__init__.py:1:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.__main__
whipper/__main__.py:4:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.common.checksum
whipper/common/checksum.py:21:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.common.encode
whipper/common/encode.py:22:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.common.cache
whipper/common/cache.py:21:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.common.program
whipper/common/program.py:25:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
whipper/common/program.py:337:30: W1632: input built-in referenced (input-builtin)
whipper/common/program.py:582:30: W1619: division w/o __future__ statement (old-division)
whipper/common/program.py:585:30: W1619: division w/o __future__ statement (old-division)
************* Module whipper.common.accurip
whipper/common/accurip.py:22:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
whipper/common/accurip.py:43:0: W1641: Implementing __eq__ without also implementing __hash__ (eq-without-hash)
************* Module whipper.common.common
whipper/common/common.py:22:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
whipper/common/common.py:78:9: W1619: division w/o __future__ statement (old-division)
whipper/common/common.py:80:35: W1619: division w/o __future__ statement (old-division)
whipper/common/common.py:80:8: W1619: division w/o __future__ statement (old-division)
whipper/common/common.py:89:9: W1619: division w/o __future__ statement (old-division)
whipper/common/common.py:91:36: W1619: division w/o __future__ statement (old-division)
whipper/common/common.py:91:9: W1619: division w/o __future__ statement (old-division)
whipper/common/common.py:93:40: W1619: division w/o __future__ statement (old-division)
whipper/common/common.py:93:35: W1619: division w/o __future__ statement (old-division)
whipper/common/common.py:93:8: W1619: division w/o __future__ statement (old-division)
whipper/common/common.py:123:12: W1619: division w/o __future__ statement (old-division)
whipper/common/common.py:127:14: W1619: division w/o __future__ statement (old-division)
************* Module whipper.common.config
whipper/common/config.py:21:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.common.renamer
whipper/common/renamer.py:21:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
whipper/common/renamer.py:160:0: W1641: Implementing __eq__ without also implementing __hash__ (eq-without-hash)
whipper/common/renamer.py:186:0: W1641: Implementing __eq__ without also implementing __hash__ (eq-without-hash)
************* Module whipper.common.path
whipper/common/path.py:21:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.common.directory
whipper/common/directory.py:21:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.common.mbngs
whipper/common/mbngs.py:24:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.common.task
whipper/common/task.py:4:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.common.drive
whipper/common/drive.py:21:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.image.image
whipper/image/image.py:25:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
whipper/image/image.py:170:18: W1619: division w/o __future__ statement (old-division)
************* Module whipper.image.cue
whipper/image/cue.py:27:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.image.table
whipper/image/table.py:25:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
whipper/image/table.py:423:50: W1619: division w/o __future__ statement (old-division)
************* Module whipper.image.toc
whipper/image/toc.py:27:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.program.cdparanoia
whipper/program/cdparanoia.py:21:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
whipper/program/cdparanoia.py:128:22: W1619: division w/o __future__ statement (old-division)
whipper/program/cdparanoia.py:182:22: W1619: division w/o __future__ statement (old-division)
whipper/program/cdparanoia.py:197:36: W1619: division w/o __future__ statement (old-division)
whipper/program/cdparanoia.py:341:23: W1619: division w/o __future__ statement (old-division)
whipper/program/cdparanoia.py:371:20: W1619: division w/o __future__ statement (old-division)
whipper/program/cdparanoia.py:390:21: W1619: division w/o __future__ statement (old-division)
************* Module whipper.program.sox
whipper/program/sox.py:1:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.program.utils
whipper/program/utils.py:1:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.program.arc
whipper/program/arc.py:1:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.program.flac
whipper/program/flac.py:1:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.program.cdrdao
whipper/program/cdrdao.py:1:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
whipper/program/cdrdao.py:130:32: W1619: division w/o __future__ statement (old-division)
************* Module whipper.program.soxi
whipper/program/soxi.py:1:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.result.result
whipper/result/result.py:21:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.result.logger
whipper/result/logger.py:1:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.test.test_common_cache
whipper/test/test_common_cache.py:4:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.test.test_common_config
whipper/test/test_common_config.py:4:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.test.test_program_sox
whipper/test/test_program_sox.py:3:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.test.test_common_program
whipper/test/test_common_program.py:5:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.test.test_common_mbngs
whipper/test/test_common_mbngs.py:4:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.test.test_image_table
whipper/test/test_image_table.py:4:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.test.test_common_directory
whipper/test/test_common_directory.py:4:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.test.test_program_cdparanoia
whipper/test/test_program_cdparanoia.py:4:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.test.test_common_path
whipper/test/test_common_path.py:4:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.test.test_image_toc
whipper/test/test_image_toc.py:4:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.test.test_command_mblookup
whipper/test/test_command_mblookup.py:4:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.test.common
whipper/test/common.py:4:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.test.test_image_cue
whipper/test/test_image_cue.py:4:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.test.test_common_renamer
whipper/test/test_common_renamer.py:4:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.test.test_common_common
whipper/test/test_common_common.py:4:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.test.test_common_drive
whipper/test/test_common_drive.py:4:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.test.test_program_cdrdao
whipper/test/test_program_cdrdao.py:4:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.test.test_common_accurip
whipper/test/test_common_accurip.py:4:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.test.test_program_soxi
whipper/test/test_program_soxi.py:3:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.extern.asyncsub
whipper/extern/asyncsub.py:6:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
whipper/extern/asyncsub.py:151:27: W1619: division w/o __future__ statement (old-division)
************* Module whipper.extern.freedb
whipper/extern/freedb.py:74:4: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.extern.task.task
whipper/extern/task/task.py:21:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
whipper/extern/task/task.py:335:8: W1622: Called a next() method on an object (next-method-called)
whipper/extern/task/task.py:337:4: W1653: next method defined (next-method-defined)
whipper/extern/task/task.py:403:4: W1653: next method defined (next-method-defined)
whipper/extern/task/task.py:429:25: W1619: division w/o __future__ statement (old-division)
whipper/extern/task/task.py:433:25: W1619: division w/o __future__ statement (old-division)
************* Module whipper.command.image
whipper/command/image.py:21:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.command.cd
whipper/command/cd.py:21:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.command.accurip
whipper/command/accurip.py:21:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.command.mblookup
whipper/command/mblookup.py:1:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.command.offset
whipper/command/offset.py:21:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.command.basecommand
whipper/command/basecommand.py:4:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.command.main
whipper/command/main.py:4:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)
************* Module whipper.command.drive
whipper/command/drive.py:21:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import)

@ArchangeGabriel ArchangeGabriel mentioned this pull request Sep 29, 2019
Closed
@jwflory
Copy link

jwflory commented Sep 29, 2019

As Travis CI needs a specific Python version set, I've arbitrarily chosen 3.5.

Hi – to add context, Red Hat is standardizing on Python 3.6 for new versions of CentOS/RHEL. It makes packaging easier for a large audience if you choose Python 3.6. I imagine that version branch of Python will also last a while since RHEL/CentOS 7 is standardizing on it. (CentOS/RHEL 8 are designed to swap Python versions more easily but I guess it will be a long, long, long time before we see several v8+ installs out there.)

Food for thought!

@JoeLametta
Copy link
Collaborator

@jwflory Thanks for the information, I think the code in this branch should run under Python 3.5 and 3.6 (maybe even 3.7) without any issues. Right now I'm not targeting Python 3.6 only because both Ubuntu 16.04 (LTS) and Debian stretch (oldstable), which are still supported, only provide Python 3.5.

@JoeLametta
Copy link
Collaborator

Rebased on develop and squashed commits.

@JoeLametta JoeLametta mentioned this pull request Oct 30, 2019
Merged
@JoeLametta
Copy link
Collaborator

Rebased on develop, merged the patch from @mtdcr to port the accuraterip-checksum extension to Python 3. I've fixed a TypeError and some warnings too.

@JoeLametta
Copy link
Collaborator

Warnings left:

/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/ruamel/yaml/representer.py:1017: ResourceWarning: unclosed file <_io.FileIO name=15 mode='wb' closefd=True>
  node_key = self.represent_key(item_key)
/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/ruamel/yaml/representer.py:1017: ResourceWarning: unclosed file <_io.FileIO name=16 mode='wb' closefd=True>
  node_key = self.represent_key(item_key)
/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/ruamel/yaml/representer.py:1017: ResourceWarning: unclosed file <_io.FileIO name=17 mode='wb' closefd=True>
  node_key = self.represent_key(item_key)
/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/ruamel/yaml/representer.py:1017: ResourceWarning: unclosed file <_io.FileIO name=18 mode='wb' closefd=True>
  node_key = self.represent_key(item_key)
.

Failure left:

FAIL: testDuration (whipper.test.test_image_toc.CapitalMergeTestCase)
testDuration
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/defer.py", line 151, in maybeDeferred
    result = f(*args, **kw)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/utils.py", line 221, in runWithWarningsSuppressed
    reraise(exc_info[1], exc_info[2])
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/python/compat.py", line 464, in reraise
    raise exception.with_traceback(traceback)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/utils.py", line 217, in runWithWarningsSuppressed
    result = f(*a, **kw)
  File "/home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py", line 271, in testDuration
    self.assertEqual(self.table.getFrameLength(), 173530)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/trial/_synctest.py", line 432, in assertEqual
    super(_Assertions, self).assertEqual(first, second, msg)
  File "/opt/python/3.5.6/lib/python3.5/unittest/case.py", line 829, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/opt/python/3.5.6/lib/python3.5/unittest/case.py", line 822, in _baseAssertEqual
    raise self.failureException(msg)
twisted.trial.unittest.FailTest: 184930 != 173530

----------------------------------------------------------------------
Ran 110 tests in 6.344s

FAILED (failures=1)

@JoeLametta JoeLametta changed the title WIP: Initial pass on python 3 port WIP: Python 3 port Oct 31, 2019
@JoeLametta
Copy link
Collaborator

I think I've found the cause of the last remaining test failure. Fix provided in commit 29bba44.

@Freso @MerlijnWajer could you please ACK this one?

Freso
Freso reviewed Nov 4, 2019
View reviewed changes
Copy link
Member

@Freso Freso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t see any outright issues with this, even if I have some questions for some of the changes made here. I’d be fine with also merging this as-is and fixing up other issues later though.

.travis.yml Show resolved Hide resolved
whipper/common/program.py Show resolved Hide resolved
whipper/image/table.py Show resolved Hide resolved
whipper/image/toc.py Outdated Show resolved Hide resolved
whipper/image/cue.py
frameOffset = frames \
+ seconds * common.FRAMES_PER_SECOND \
+ minutes * common.FRAMES_PER_SECOND * 60
frameOffset = int(frames
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the explicit int() casting needed here?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know but I think it's unneeded...

whipper/extern/asyncsub.py Show resolved Hide resolved
@JoeLametta
Copy link
Collaborator

Don't know if the three remaining warnings are caused by whipper: they seem to come from the ruamel module...

@JoeLametta
Copy link
Collaborator

JoeLametta commented Nov 6, 2019 •
edited
Loading

After manual testing of this port, I've found a regression (everything else appears to be working fine).
Output:

$ WHIPPER_DEBUG=DEBUG WHIPPER_LOGFILE=whipper.log whipper image verify *.cue
Traceback (most recent call last):              
  File "/usr/local/bin/whipper", line 11, in <module>
    load_entry_point('whipper==0.7.4.dev124+gc981ce2.d20191106', 'console_scripts', 'whipper')()
  File "/usr/local/lib/python3.6/dist-packages/whipper-0.7.4.dev124+gc981ce2.d20191106-py3.6-linux-x86_64.egg/whipper/command/main.py", line 43, in main
    ret = cmd.do()
  File "/usr/local/lib/python3.6/dist-packages/whipper-0.7.4.dev124+gc981ce2.d20191106-py3.6-linux-x86_64.egg/whipper/command/basecommand.py", line 139, in do
    return self.cmd.do()
  File "/usr/local/lib/python3.6/dist-packages/whipper-0.7.4.dev124+gc981ce2.d20191106-py3.6-linux-x86_64.egg/whipper/command/basecommand.py", line 139, in do
    return self.cmd.do()
  File "/usr/local/lib/python3.6/dist-packages/whipper-0.7.4.dev124+gc981ce2.d20191106-py3.6-linux-x86_64.egg/whipper/command/image.py", line 61, in do
    verified = prog.verifyImage(runner, cueImage.table)
  File "/usr/local/lib/python3.6/dist-packages/whipper-0.7.4.dev124+gc981ce2.d20191106-py3.6-linux-x86_64.egg/whipper/common/program.py", line 561, in verifyImage
    responses = accurip.get_db_entry(table.accuraterip_path())
  File "/usr/local/lib/python3.6/dist-packages/whipper-0.7.4.dev124+gc981ce2.d20191106-py3.6-linux-x86_64.egg/whipper/image/table.py", line 828, in accuraterip_path
    discId1, discId2 = self.accuraterip_ids()
  File "/usr/local/lib/python3.6/dist-packages/whipper-0.7.4.dev124+gc981ce2.d20191106-py3.6-linux-x86_64.egg/whipper/image/table.py", line 822, in accuraterip_ids
    discId1 &= 0xffffffff
TypeError: unsupported operand type(s) for &=: 'float' and 'int'

Debug log:

DEBUG:whipper.common.config:loaded 1 sections from config file
DEBUG:whipper.common.config:loaded 1 sections from config file
DEBUG:whipper.common.config:loaded 1 sections from config file
DEBUG:whipper.common.config:loaded 1 sections from config file
DEBUG:whipper.common.config:loaded 1 sections from config file
DEBUG:whipper.common.config:loaded 1 sections from config file
DEBUG:whipper.common.config:loaded 1 sections from config file
DEBUG:whipper.image.table:set logName
INFO:whipper.image.cue:parsing .cue file 'HTOA Test CD - DAE Drive Features Database.cue'
DEBUG:whipper.image.cue:found track 1
DEBUG:whipper.image.cue:found index 0 of track <Track 01> in './00. HTOA Test CD - Hidden Track One Audio.flac':0
DEBUG:whipper.image.cue:found index 1 of track <Track 01> in './01. HTOA Test CD - Music (In Pregap and Track).flac':0
DEBUG:whipper.image.image:setup image start
DEBUG:whipper.image.image:schedule scan of audio length of './00. HTOA Test CD - Hidden Track One Audio.flac'
DEBUG:whipper.image.image:verifying track 1
DEBUG:whipper.image.image:schedule scan of audio length of './01. HTOA Test CD - Music (In Pregap and Track).flac'
DEBUG:whipper.image.image:verifying image
DEBUG:whipper.extern.task.task:run task <whipper.image.image.ImageVerifyTask object at 0x7f77d37b3a20>
DEBUG:whipper.extern.task.task:Adding listener <whipper.extern.task.task.SyncRunner object at 0x7f77d37b36a0>
DEBUG:whipper.extern.task.task:run loop
DEBUG:whipper.extern.task.task:start task <whipper.image.image.ImageVerifyTask object at 0x7f77d37b3a20>
DEBUG:whipper.extern.task.task:MultiSeparateTask.start()
DEBUG:whipper.extern.task.task:starting
DEBUG:whipper.extern.task.task:notifying progress: 0.0 on 'Checking tracks'
DEBUG:whipper.extern.task.task:MultiSeparateTask.next()
DEBUG:whipper.extern.task.task:BaseMultiTask.next(): starting task 1 of 2: <whipper.program.soxi.AudioLengthTask object at 0x7f77d37b35c0>
DEBUG:whipper.extern.task.task:Adding listener <whipper.image.image.ImageVerifyTask object at 0x7f77d37b3a20>
DEBUG:whipper.extern.task.task:starting
DEBUG:whipper.extern.task.task:notifying progress: 0.0 on 'Getting length of audio track (1 of 2) ...'
DEBUG:whipper.extern.task.task:notifying progress: 0.0 on 'Getting length of audio track'
DEBUG:whipper.common.task:started ['soxi', '-s', './00. HTOA Test CD - Hidden Track One Audio.flac'] with pid 31609
DEBUG:whipper.extern.task.task:BaseMultiTask.next(): started task 1 of 2: <whipper.program.soxi.AudioLengthTask object at 0x7f77d37b35c0>
DEBUG:whipper.common.task:read from stdout: b'8305500\n'
DEBUG:whipper.common.task:return code was 0
DEBUG:whipper.extern.task.task:notifying progress: 1.0 on 'Getting length of audio track (1 of 2) ...'
DEBUG:whipper.extern.task.task:notifying progress: 1.0 on 'Getting length of audio track'
DEBUG:whipper.extern.task.task:stopping
DEBUG:whipper.extern.task.task:reset runner to None
DEBUG:whipper.extern.task.task:BaseMultiTask.stopped: task <whipper.program.soxi.AudioLengthTask object at 0x7f77d37b35c0> (1 of 2)
DEBUG:whipper.extern.task.task:BaseMultiTask.stopped: pick next task
DEBUG:whipper.extern.task.task:MultiSeparateTask.next()
DEBUG:whipper.extern.task.task:BaseMultiTask.next(): starting task 2 of 2: <whipper.program.soxi.AudioLengthTask object at 0x7f77d37b35f8>
DEBUG:whipper.extern.task.task:Adding listener <whipper.image.image.ImageVerifyTask object at 0x7f77d37b3a20>
DEBUG:whipper.extern.task.task:starting
DEBUG:whipper.extern.task.task:notifying progress: 0.0 on 'Getting length of audio track (2 of 2) ...'
DEBUG:whipper.extern.task.task:notifying progress: 0.0 on 'Getting length of audio track'
DEBUG:whipper.common.task:started ['soxi', '-s', './01. HTOA Test CD - Music (In Pregap and Track).flac'] with pid 31610
DEBUG:whipper.extern.task.task:BaseMultiTask.next(): started task 2 of 2: <whipper.program.soxi.AudioLengthTask object at 0x7f77d37b35f8>
DEBUG:whipper.common.task:read from stdout: b'10324104\n'
DEBUG:whipper.common.task:return code was 0
DEBUG:whipper.extern.task.task:notifying progress: 1.0 on 'Getting length of audio track (2 of 2) ...'
DEBUG:whipper.extern.task.task:notifying progress: 1.0 on 'Getting length of audio track'
DEBUG:whipper.extern.task.task:stopping
DEBUG:whipper.extern.task.task:reset runner to None
DEBUG:whipper.extern.task.task:BaseMultiTask.stopped: task <whipper.program.soxi.AudioLengthTask object at 0x7f77d37b35f8> (2 of 2)
DEBUG:whipper.extern.task.task:BaseMultiTask.stopped: all tasks done
DEBUG:whipper.extern.task.task:stopping
DEBUG:whipper.extern.task.task:reset runner to None
DEBUG:whipper.extern.task.task:stopped task <whipper.image.image.ImageVerifyTask object at 0x7f77d37b3a20>
DEBUG:whipper.extern.task.task:done running task <whipper.image.image.ImageVerifyTask object at 0x7f77d37b3a20>
DEBUG:whipper.image.image:verified image
DEBUG:whipper.image.table:set logName
DEBUG:whipper.image.image:setup image done
DEBUG:whipper.image.table:set logName
INFO:whipper.image.cue:parsing .cue file 'HTOA Test CD - DAE Drive Features Database.cue'
DEBUG:whipper.image.cue:found track 1
DEBUG:whipper.image.cue:found index 0 of track <Track 01> in './00. HTOA Test CD - Hidden Track One Audio.flac':0
DEBUG:whipper.image.cue:found index 1 of track <Track 01> in './01. HTOA Test CD - Music (In Pregap and Track).flac':0
DEBUG:whipper.image.image:schedule scan of audio length of './00. HTOA Test CD - Hidden Track One Audio.flac'
DEBUG:whipper.image.image:verifying track 1
DEBUG:whipper.image.image:schedule scan of audio length of './01. HTOA Test CD - Music (In Pregap and Track).flac'
DEBUG:whipper.extern.task.task:run task <whipper.image.image.ImageVerifyTask object at 0x7f77d37b30b8>
DEBUG:whipper.extern.task.task:Adding listener <whipper.extern.task.task.SyncRunner object at 0x7f77d37b36a0>
DEBUG:whipper.extern.task.task:run loop
DEBUG:whipper.extern.task.task:start task <whipper.image.image.ImageVerifyTask object at 0x7f77d37b30b8>
DEBUG:whipper.extern.task.task:MultiSeparateTask.start()
DEBUG:whipper.extern.task.task:starting
DEBUG:whipper.extern.task.task:notifying progress: 0.0 on 'Checking tracks'
DEBUG:whipper.extern.task.task:MultiSeparateTask.next()
DEBUG:whipper.extern.task.task:BaseMultiTask.next(): starting task 1 of 2: <whipper.program.soxi.AudioLengthTask object at 0x7f77d37b3e80>
DEBUG:whipper.extern.task.task:Adding listener <whipper.image.image.ImageVerifyTask object at 0x7f77d37b30b8>
DEBUG:whipper.extern.task.task:starting
DEBUG:whipper.extern.task.task:notifying progress: 0.0 on 'Getting length of audio track (1 of 2) ...'
DEBUG:whipper.extern.task.task:notifying progress: 0.0 on 'Getting length of audio track'
DEBUG:whipper.common.task:started ['soxi', '-s', './00. HTOA Test CD - Hidden Track One Audio.flac'] with pid 31611
DEBUG:whipper.extern.task.task:BaseMultiTask.next(): started task 1 of 2: <whipper.program.soxi.AudioLengthTask object at 0x7f77d37b3e80>
DEBUG:whipper.common.task:read from stdout: b'8305500\n'
DEBUG:whipper.common.task:return code was 0
DEBUG:whipper.extern.task.task:notifying progress: 1.0 on 'Getting length of audio track (1 of 2) ...'
DEBUG:whipper.extern.task.task:notifying progress: 1.0 on 'Getting length of audio track'
DEBUG:whipper.extern.task.task:stopping
DEBUG:whipper.extern.task.task:reset runner to None
DEBUG:whipper.extern.task.task:BaseMultiTask.stopped: task <whipper.program.soxi.AudioLengthTask object at 0x7f77d37b3e80> (1 of 2)
DEBUG:whipper.extern.task.task:BaseMultiTask.stopped: pick next task
DEBUG:whipper.extern.task.task:MultiSeparateTask.next()
DEBUG:whipper.extern.task.task:BaseMultiTask.next(): starting task 2 of 2: <whipper.program.soxi.AudioLengthTask object at 0x7f77d37b3cc0>
DEBUG:whipper.extern.task.task:Adding listener <whipper.image.image.ImageVerifyTask object at 0x7f77d37b30b8>
DEBUG:whipper.extern.task.task:starting
DEBUG:whipper.extern.task.task:notifying progress: 0.0 on 'Getting length of audio track (2 of 2) ...'
DEBUG:whipper.extern.task.task:notifying progress: 0.0 on 'Getting length of audio track'
DEBUG:whipper.common.task:started ['soxi', '-s', './01. HTOA Test CD - Music (In Pregap and Track).flac'] with pid 31612
DEBUG:whipper.extern.task.task:BaseMultiTask.next(): started task 2 of 2: <whipper.program.soxi.AudioLengthTask object at 0x7f77d37b3cc0>
DEBUG:whipper.common.task:read from stdout: b'10324104\n'
DEBUG:whipper.common.task:return code was 0
DEBUG:whipper.extern.task.task:notifying progress: 1.0 on 'Getting length of audio track (2 of 2) ...'
DEBUG:whipper.extern.task.task:notifying progress: 1.0 on 'Getting length of audio track'
DEBUG:whipper.extern.task.task:stopping
DEBUG:whipper.extern.task.task:reset runner to None
DEBUG:whipper.extern.task.task:BaseMultiTask.stopped: task <whipper.program.soxi.AudioLengthTask object at 0x7f77d37b3cc0> (2 of 2)
DEBUG:whipper.extern.task.task:BaseMultiTask.stopped: all tasks done
DEBUG:whipper.extern.task.task:stopping
DEBUG:whipper.extern.task.task:reset runner to None
DEBUG:whipper.extern.task.task:stopped task <whipper.image.image.ImageVerifyTask object at 0x7f77d37b30b8>
DEBUG:whipper.extern.task.task:done running task <whipper.image.image.ImageVerifyTask object at 0x7f77d37b30b8>

Code of the relevant section:

whipper/whipper/image/table.py

Lines 799 to 825 in f6dcc08

def accuraterip_ids(self):
"""
returns both AccurateRip disc ids as a tuple of 8-char
hexadecimal strings (discid1, discid2)
"""
# AccurateRip does not take into account data tracks,
# but does count the data track to determine the leadout offset
discId1 = 0
discId2 = 0
for track in self.tracks:
if not track.audio:
continue
offset = self.getTrackStart(track.number)
discId1 += offset
discId2 += (offset or 1) * track.number
# also add end values, where leadout offset is one past the end
# of the last track
offset = self.getTrackEnd(self.tracks[-1].number) + 1
discId1 += offset
discId2 += offset * (self.getAudioTracks() + 1)
discId1 &= 0xffffffff
discId2 &= 0xffffffff
return "%08x" % discId1, "%08x" % discId2

It seems that the offset is a float (int.0).
The value of the offset comes either from offset = self.getTrackStart(track.number), relevant code here:

whipper/whipper/image/table.py

Lines 194 to 203 in f6dcc08

def getTrackStart(self, number):
"""
:param number: the track number, 1-based
:type number: int
:returns: the start of the given track number's index 1, in CD frames
:rtype: int
"""
track = self.tracks[number - 1]
return track.getIndex(1).absolute

or from offset = self.getTrackEnd(self.tracks[-1].number) + 1, relevant code here:

whipper/whipper/image/table.py

Lines 205 to 227 in f6dcc08

def getTrackEnd(self, number):
"""
:param number: the track number, 1-based
:type number: int
:returns: the end of the given track number (ie index 1 of next track)
:rtype: int
"""
# default to end of disc
end = self.leadout - 1
# if not last track, calculate it from the next track
if number < len(self.tracks):
end = self.tracks[number].getIndex(1).absolute - 1
# if on a session border, subtract the session leadin
thisTrack = self.tracks[number - 1]
nextTrack = self.tracks[number]
if int(nextTrack.session or 1) > int(thisTrack.session or 1):
gap = self._getSessionGap(nextTrack.session)
end -= gap
return end

In both cases what's alarming is that the float value seems to originate from the self.tracks[number].getIndex(index).absolute call. I don't know why that one is a float but it should be an int.
If I run the same command using whipper's code from develop branch (commit f740a0e) against the same files, it completes successfully.

This can be replicated using the files included in this ZIP archive but I don't think it depends on these specific files.

@JoeLametta JoeLametta mentioned this pull request Nov 7, 2019
Closed
@MerlijnWajer
Copy link
Collaborator

Regarding 29bba44 - I would personally not use variable name or 1 and just make it more verbose so it's easier to understand, but I think it looks OK otherwise.

@JoeLametta
Copy link
Collaborator

Regarding 29bba44 - I would personally not use variable name or 1 and just make it more verbose so it's easier to understand, but I think it looks OK otherwise.

I've just rebased it and added both a detailed python comment and a rich commit description (c71924e).

ddevault and others added 2 commits November 26, 2019 18:46
@ddevault @JoeLametta
Initial pass on python 3 port
64dd9d8 
Given the imminent end-of-life for Python 2, I didn't bother making the
codebase compatible with both.

Signed-off-by: Drew DeVault <sir@cmpwn.com>
@JoeLametta
Address Freso's comment
b2d0664 
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
mtdcr and others added 6 commits November 26, 2019 18:46
@mtdcr @JoeLametta
accuraterip-checksum.c: Port to Python 3
8446c29 
Accuraterip-checksum extension will be Python 3 only (JoeLametta).

Co-authored-by: JoeLametta <JoeLametta@users.noreply.github.com>
Signed-off-by: Andreas Oberritter <obi@saftware.de>
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
@JoeLametta
Address errors, improvements, formatting
35201d5 
- Removed unused code not portable due to buffer() use
- raw_input() does not exist in Python 3
- Fixed octal constant syntax for Python 3
- Fixed TypeError
- Replace if not exists: makedirs(path) with single call: using makedirs(path, exist_ok=True)
- Class inherits from object, can be safely removed from bases in python3: pylint's useless-object-inheritance (W0235) check

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
@JoeLametta
Address test failures
fff3014 
More details about the fix to the testDuration failure (regression):

```
FAIL: testDuration (whipper.test.test_image_toc.CapitalMergeTestCase)
testDuration
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/defer.py", line 151, in maybeDeferred
    result = f(*args, **kw)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/utils.py", line 221, in runWithWarningsSuppressed
    reraise(exc_info[1], exc_info[2])
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/python/compat.py", line 464, in reraise
    raise exception.with_traceback(traceback)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/internet/utils.py", line 217, in runWithWarningsSuppressed
    result = f(*a, **kw)
  File "/home/travis/build/whipper-team/whipper/whipper/test/test_image_toc.py", line 271, in testDuration
    self.assertEqual(self.table.getFrameLength(), 173530)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/twisted/trial/_synctest.py", line 432, in assertEqual
    super(_Assertions, self).assertEqual(first, second, msg)
  File "/opt/python/3.5.6/lib/python3.5/unittest/case.py", line 829, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/opt/python/3.5.6/lib/python3.5/unittest/case.py", line 822, in _baseAssertEqual
    raise self.failureException(msg)
twisted.trial.unittest.FailTest: 184930 != 173530
```

The test fails because if either nextTrack.session or thisTrack.session are None the if is false and the instructions inside it aren't executed. The check for None is needed because Python 3 doesn't allow NoneType comparisons (in Python 2 that was possible).
IIRC correctly in that test nextTrack.session has value 2 while thisTrack.session is None. That means the Python 2 version evaluates the if condition to true, while the Python 3 version in the first commit does not.
With this change both of the values of nextTrack.session and thisTrack.session are compared as int (if None, the value 1 is used for the comparison - as in disc session 1).

Regression introduced in 64dd9d8.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
@JoeLametta
Address ResourceWarning warnings
bb4c25d 
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
@JoeLametta
Port (misc) offsets script to Python 3
42019ce 
I've also changed the output format a bit.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
@JoeLametta
Update README, .travis.yml and Dockerfile for Python 3
50c8cbb 
Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
@JoeLametta
Copy link
Collaborator

Even if there's a regression, I'm going to merge this one as is and will open a new issue.

@JoeLametta JoeLametta changed the title WIP: Python 3 port Python 3 port Nov 26, 2019
@JoeLametta JoeLametta mentioned this pull request Nov 26, 2019
Closed
@JoeLametta JoeLametta merged commit d0efd74 into whipper-team:develop Nov 26, 2019
@JoeLametta
Copy link
Collaborator

Closes #78.

@ddevault
Copy link
Contributor Author

🎉

archlinux-github pushed a commit to archlinux/aur that referenced this pull request Jul 30, 2022
@Freso
Update package as per changes in upstream
23f8853 
accuraterip-checksum converted to C extension:
whipper-team/whipper#274

Switch to YAML library (ruamel) for formatting log files:
whipper-team/whipper#415

Port to Python 3:
whipper-team/whipper#411
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoeLametta JoeLametta JoeLametta left review comments

@ArchangeGabriel ArchangeGabriel ArchangeGabriel left review comments

@Freso Freso Freso left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.9.0
Development

Successfully merging this pull request may close these issues.
7 participants
@ddevault @MerlijnWajer @JoeLametta @ArchangeGabriel @jwflory @Freso @mtdcr