From 0bd6500cad470d3683f176f1c5d324797c33a28d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miro=20Hron=C4=8Dok?= Date: Tue, 20 Nov 2018 08:12:20 +0100 Subject: [PATCH 1/9] Add a a test for failing sscanf See https://github.com/WoLpH/numpy-stl/issues/52 --- tests/qt-lc_numeric-reproducer | 18 ++++++++++++++++++ tests/requirements.txt | 1 + tests/test_ascii.py | 30 ++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+) create mode 100644 tests/qt-lc_numeric-reproducer diff --git a/tests/qt-lc_numeric-reproducer b/tests/qt-lc_numeric-reproducer new file mode 100644 index 0000000..cb41335 --- /dev/null +++ b/tests/qt-lc_numeric-reproducer @@ -0,0 +1,18 @@ +from __future__ import print_function + +import os.path +import sys + +from stl import mesh + +try: + from PySide2 import QtWidgets +except ImportError: + from PyQt5 import QtWidgets + +app = QtWidgets.QApplication([]) + +dir_path = os.path.dirname(os.path.realpath(__file__)) +stl_path = os.path.join(dir_path, 'stl_ascii/Star.stl') + +your_mesh = mesh.Mesh.from_file(stl_path, speedups=True) diff --git a/tests/requirements.txt b/tests/requirements.txt index 032cbaa..6de9530 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -7,6 +7,7 @@ cython pep8 py pyflakes +PyQt5 pytest pytest-cache pytest-cov diff --git a/tests/test_ascii.py b/tests/test_ascii.py index eae4075..351e26d 100644 --- a/tests/test_ascii.py +++ b/tests/test_ascii.py @@ -1,3 +1,8 @@ +import os +import pytest +import subprocess +import sys + from stl.utils import b from stl import mesh @@ -50,3 +55,28 @@ def test_scientific_notation(tmpdir, speedups): assert test_mesh.name == b(name) +@pytest.mark.skipif(sys.platform.startswith('win'), + reason='Only makes sense on Unix') +def test_use_with_qr_with_custom_locale_decimal_delimeter(): + dir_path = os.path.dirname(os.path.realpath(__file__)) + script_path = os.path.join(dir_path, 'qt-lc_numeric-reproducer') + + env = os.environ.copy() + env['LC_NUMERIC'] = 'cs_CZ.utf-8' + + prefix = tuple() + if sys.platform.startswith('linux'): + prefix = ('xvfb-run', '-d') + + cp = subprocess.run(prefix + (sys.executable, script_path), + env=env, check=True, + universal_newlines=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + + # Unable to read the file with speedups, retrying + # https://github.com/WoLpH/numpy-stl/issues/52 + sys.stdout.write(cp.stdout) + sys.stderr.write(cp.stderr) + assert 'speedups' not in cp.stdout + assert 'speedups' not in cp.stderr From 2c7d613357667853acf9f4418e75ef3070a23047 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miro=20Hron=C4=8Dok?= Date: Tue, 20 Nov 2018 08:23:17 +0100 Subject: [PATCH 2/9] Revert a workaround for the LC_NUMERIC issue Reverts efedf7aa43bace998308d3c7cb859fc05b582a00 --- stl/stl.py | 16 +++------------- tests/test_ascii.py | 7 ++++--- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/stl/stl.py b/stl/stl.py index 23da33f..c658b24 100644 --- a/stl/stl.py +++ b/stl/stl.py @@ -6,7 +6,6 @@ import enum import numpy import struct -import logging import datetime from . import base @@ -20,9 +19,6 @@ _speedups = None -logger = logging.getLogger(__name__) - - class Mode(enum.IntEnum): #: Automatically detect whether the output is a TTY, if so, write ASCII #: otherwise write BINARY @@ -328,15 +324,9 @@ def from_file(cls, filename, calculate_normals=True, fh=None, name, data = cls.load( fh, mode=mode, speedups=speedups) else: - try: - with open(filename, 'rb') as fh: - name, data = cls.load( - fh, mode=mode, speedups=speedups) - except AssertionError: # pragma: no cover - logger.warn('Unable to read the file with speedups, retrying') - with open(filename, 'rb') as fh: - name, data = cls.load( - fh, mode=Mode.ASCII, speedups=False) + with open(filename, 'rb') as fh: + name, data = cls.load( + fh, mode=mode, speedups=speedups) return cls(data, calculate_normals, name=name, speedups=speedups, **kwargs) diff --git a/tests/test_ascii.py b/tests/test_ascii.py index 351e26d..224bbb0 100644 --- a/tests/test_ascii.py +++ b/tests/test_ascii.py @@ -69,7 +69,7 @@ def test_use_with_qr_with_custom_locale_decimal_delimeter(): prefix = ('xvfb-run', '-d') cp = subprocess.run(prefix + (sys.executable, script_path), - env=env, check=True, + env=env, check=False, universal_newlines=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) @@ -78,5 +78,6 @@ def test_use_with_qr_with_custom_locale_decimal_delimeter(): # https://github.com/WoLpH/numpy-stl/issues/52 sys.stdout.write(cp.stdout) sys.stderr.write(cp.stderr) - assert 'speedups' not in cp.stdout - assert 'speedups' not in cp.stderr + assert 'File too large' not in cp.stdout + assert 'File too large' not in cp.stderr + assert cp.returncode == 0 From 6049dcf4756455a3572c301aeae2b5b523cd0b77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miro=20Hron=C4=8Dok?= Date: Tue, 20 Nov 2018 08:25:36 +0100 Subject: [PATCH 3/9] Only test LC_NUMERIC once --- tests/test_ascii.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_ascii.py b/tests/test_ascii.py index 224bbb0..3701cd8 100644 --- a/tests/test_ascii.py +++ b/tests/test_ascii.py @@ -57,7 +57,10 @@ def test_scientific_notation(tmpdir, speedups): @pytest.mark.skipif(sys.platform.startswith('win'), reason='Only makes sense on Unix') -def test_use_with_qr_with_custom_locale_decimal_delimeter(): +def test_use_with_qr_with_custom_locale_decimal_delimeter(speedups): + if not speedups: + pytest.skip('Only makes sense with speedups') + dir_path = os.path.dirname(os.path.realpath(__file__)) script_path = os.path.join(dir_path, 'qt-lc_numeric-reproducer') From 222961822b635f19de324d17013e5e8805e6d560 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miro=20Hron=C4=8Dok?= Date: Tue, 20 Nov 2018 12:56:24 +0100 Subject: [PATCH 4/9] Make PyQt5 test dependency optional, fix test for old Pythons --- tests/requirements.txt | 1 - tests/test_ascii.py | 25 +++++++++++++++---------- tox.ini | 6 ++++++ 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/tests/requirements.txt b/tests/requirements.txt index 6de9530..032cbaa 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -7,7 +7,6 @@ cython pep8 py pyflakes -PyQt5 pytest pytest-cache pytest-cov diff --git a/tests/test_ascii.py b/tests/test_ascii.py index 3701cd8..72bdf2a 100644 --- a/tests/test_ascii.py +++ b/tests/test_ascii.py @@ -71,16 +71,21 @@ def test_use_with_qr_with_custom_locale_decimal_delimeter(speedups): if sys.platform.startswith('linux'): prefix = ('xvfb-run', '-d') - cp = subprocess.run(prefix + (sys.executable, script_path), - env=env, check=False, - universal_newlines=True, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE) + p = subprocess.Popen(prefix + (sys.executable, script_path), + env=env, + universal_newlines=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + out, err = p.communicate() # Unable to read the file with speedups, retrying # https://github.com/WoLpH/numpy-stl/issues/52 - sys.stdout.write(cp.stdout) - sys.stderr.write(cp.stderr) - assert 'File too large' not in cp.stdout - assert 'File too large' not in cp.stderr - assert cp.returncode == 0 + sys.stdout.write(out) + sys.stderr.write(err) + + if 'No module named' in out: + pytest.skip('Optional dependency PyQt5 or PySide2 not installed') + + assert 'File too large' not in out + assert 'File too large' not in err + assert p.returncode == 0 diff --git a/tox.ini b/tox.ini index f8a1057..6506208 100644 --- a/tox.ini +++ b/tox.ini @@ -34,3 +34,9 @@ changedir=docs commands= sphinx-build -b html -d {envtmpdir}/doctrees . {envtmpdir}/html # sphinx-build -W -b html -d {envtmpdir}/doctrees . {envtmpdir}/html + +[testenv:py36-nix] +# one optional test has PyQt5 dep, only test that once +deps = + -rtests/requirements.txt + PyQt5 From 9c553cf14f10ac055f8208f9046210ef2b40fbdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miro=20Hron=C4=8Dok?= Date: Tue, 20 Nov 2018 13:00:34 +0100 Subject: [PATCH 5/9] Typo in test name --- tests/test_ascii.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_ascii.py b/tests/test_ascii.py index 72bdf2a..3f72acb 100644 --- a/tests/test_ascii.py +++ b/tests/test_ascii.py @@ -57,7 +57,7 @@ def test_scientific_notation(tmpdir, speedups): @pytest.mark.skipif(sys.platform.startswith('win'), reason='Only makes sense on Unix') -def test_use_with_qr_with_custom_locale_decimal_delimeter(speedups): +def test_use_with_qt_with_custom_locale_decimal_delimeter(speedups): if not speedups: pytest.skip('Only makes sense with speedups') From b34016b8078b7097fe05fca51344e8da1a9c346c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miro=20Hron=C4=8Dok?= Date: Tue, 20 Nov 2018 13:47:02 +0100 Subject: [PATCH 6/9] Support old xvfb-run --- tests/test_ascii.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_ascii.py b/tests/test_ascii.py index 3f72acb..d19bb6c 100644 --- a/tests/test_ascii.py +++ b/tests/test_ascii.py @@ -69,7 +69,7 @@ def test_use_with_qt_with_custom_locale_decimal_delimeter(speedups): prefix = tuple() if sys.platform.startswith('linux'): - prefix = ('xvfb-run', '-d') + prefix = ('xvfb-run', '-a') p = subprocess.Popen(prefix + (sys.executable, script_path), env=env, From cfb701131918bd3766c896f28c66c771c5d61835 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miro=20Hron=C4=8Dok?= Date: Tue, 20 Nov 2018 13:45:35 +0100 Subject: [PATCH 7/9] Set LC_NUMERIC to C for sscanf operations --- stl/_speedups.pyx | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/stl/_speedups.pyx b/stl/_speedups.pyx index 1d900d1..10dd8b1 100644 --- a/stl/_speedups.pyx +++ b/stl/_speedups.pyx @@ -8,6 +8,17 @@ ELSE: cdef extern from 'unistd.h': int dup(int fd) + cdef extern from 'locale.h': + struct locale_t: + pass + + locale_t uselocale(locale_t __dataset) + locale_t newlocale(int __category_mask, const char *__locale, + locale_t __base) + void freelocale(locale_t __dataset) + + enum: LC_NUMERIC_MASK + import numpy as np cimport numpy as np @@ -89,6 +100,13 @@ def ascii_read(fh, buf): cdef size_t pos = 0 cdef State state + + IF UNAME_SYSNAME != 'Windows': + cdef locale_t new_locale = newlocale(LC_NUMERIC_MASK, 'C', + NULL) + cdef locale_t old_locale = uselocale(new_locale) + freelocale(new_locale) + try: state.size = len(buf) memcpy(state.buf, buf, state.size) @@ -147,6 +165,11 @@ def ascii_read(fh, buf): fclose(state.fp) fh.seek(pos, SEEK_SET) + IF UNAME_SYSNAME != 'Windows': + uselocale(old_locale) + freelocale(old_locale) + + def ascii_write(fh, name, np.ndarray[Facet, mode = 'c', cast=True] arr): cdef FILE* fp cdef Facet* facet = arr.data From ffefbb995d3947d55a8241f2a4be674a3dd4b66b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miro=20Hron=C4=8Dok?= Date: Tue, 20 Nov 2018 21:03:49 +0100 Subject: [PATCH 8/9] locale_t is a typedef struct --- stl/_speedups.pyx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/_speedups.pyx b/stl/_speedups.pyx index 10dd8b1..8e6a410 100644 --- a/stl/_speedups.pyx +++ b/stl/_speedups.pyx @@ -9,7 +9,7 @@ ELSE: int dup(int fd) cdef extern from 'locale.h': - struct locale_t: + ctypedef struct locale_t: pass locale_t uselocale(locale_t __dataset) From 6736fa3949b581b501762c3040628ffe9a9fd1a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miro=20Hron=C4=8Dok?= Date: Wed, 21 Nov 2018 18:56:48 +0100 Subject: [PATCH 9/9] Only do locale stuff on Linux --- stl/_speedups.pyx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/stl/_speedups.pyx b/stl/_speedups.pyx index 8e6a410..d9fea13 100644 --- a/stl/_speedups.pyx +++ b/stl/_speedups.pyx @@ -8,6 +8,7 @@ ELSE: cdef extern from 'unistd.h': int dup(int fd) +IF UNAME_SYSNAME == 'Linux': cdef extern from 'locale.h': ctypedef struct locale_t: pass @@ -101,7 +102,7 @@ def ascii_read(fh, buf): cdef State state - IF UNAME_SYSNAME != 'Windows': + IF UNAME_SYSNAME == 'Linux': cdef locale_t new_locale = newlocale(LC_NUMERIC_MASK, 'C', NULL) cdef locale_t old_locale = uselocale(new_locale) @@ -165,7 +166,7 @@ def ascii_read(fh, buf): fclose(state.fp) fh.seek(pos, SEEK_SET) - IF UNAME_SYSNAME != 'Windows': + IF UNAME_SYSNAME == 'Linux': uselocale(old_locale) freelocale(old_locale)