bootphon / phonemizer

Simple text to phones converter for multiple languages
https://bootphon.github.io/phonemizer/
GNU General Public License v3.0
1.19k stars 166 forks source link

Dockerfile build fix #45

Closed alongreyber closed 4 years ago

alongreyber commented 4 years ago

When I cloned the repository and ran docker build -t phonemizer . I received the following error:

Traceback (most recent call last):
  File "/usr/lib/python3.6/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/lib/python3.6/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/usr/lib/python3/dist-packages/pytest.py", line 13, in <module>
    from _pytest.fixtures import fixture, yield_fixture
  File "/usr/lib/python3/dist-packages/_pytest/fixtures.py", line 842, in <module>
    class FixtureFunctionMarker(object):
  File "/usr/lib/python3/dist-packages/_pytest/fixtures.py", line 844, in FixtureFunctionMarker
    params = attr.ib(convert=attr.converters.optional(tuple))
TypeError: attrib() got an unexpected keyword argument 'convert'

This was caused by an outdated version of pytest that comes with installing python3-pytest through apt. I fixed this by installing pytest through pip. After that I had four failing tests:

=========================== short test summary info ============================
FAILED test/test_espeak.py::test_path_bad - Failed: DID NOT RAISE <class 'Run...
FAILED test/test_espeak.py::test_path_venv - TypeError: str expected, not Non...
FAILED test/test_festival.py::test_path_bad - Failed: DID NOT RAISE <class 'R...
FAILED test/test_festival.py::test_path_venv - TypeError: str expected, not N...
======================== 4 failed, 145 passed in 41.45s ========================

These were caused by the tests looking for python executable but only python3 is available when installing python via apt. I modified the Dockerfile to create a symlink to the python executable rather than modifying the tests themselves.

You should be able to confirm that docker build -t phonemizer . works with the modified Dockerfile. Let me know if there is a different way you would like me to fix either of these issues!

codecov[bot] commented 4 years ago

Codecov Report

Merging #45 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #45   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           14        14           
  Lines          676       676           
=========================================
  Hits           676       676           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 267960a...46e5641. Read the comment docs.

mmmaat commented 4 years ago

Great thank you! I'm merging.