Tidy up the propagation of exceptions from UDUNITS2 #136

pelson · 2019-01-21T17:00:33Z

Modernises the exception raising when propagating from udunits2.

In python 3 we have a syntax to allow us to control whether to show multiple tracebacks (super useful for the case of exceptions being raised in multiple threads). This functionality is the default in python 3. Essentially, the following code:

def foo():
    raise ValueError('Inside foo')

def bar():
    try:
        foo()
    except Exception as err:
        raise ValueError('bar')

bar()

$ python2 raise_from.py 
Traceback (most recent call last):
  File "raise_from.py", line 15, in <module>
    bar()
  File "raise_from.py", line 11, in bar
    raise ValueError('bar')
ValueError: bar

python3 raise_from.py 
Traceback (most recent call last):
  File "raise_from.py", line 9, in bar
    foo()
  File "raise_from.py", line 2, in foo
    raise ValueError('Inside foo')
ValueError: Inside foo

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "raise_from.py", line 15, in <module>
    bar()
  File "raise_from.py", line 11, in bar
    raise ValueError('bar')
ValueError: bar

The same is true for existing cf_units:

python2 -c "import cf_units as u; u.Unit('2^m')"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "cf_units/__init__.py", line 836, in __init__
    self._propogate_error('Failed to parse unit "%s"' % unit, e)
  File "cf_units/__init__.py", line 871, in _propogate_error
    raise ValueError('[%s] %s%s' % (ud_err.status_msg(), msg, error_msg))
ValueError: [UT_SUCCESS] Failed to parse unit "2^m"

python3 -c "import cf_units as u; u.Unit('2^m')"
Traceback (most recent call last):
  File "/Users/pelson/dev/scitools/cf-units/cf_units/__init__.py", line 834, in __init__
    ut_unit = _ud.parse(_ud_system, unit.encode('ascii'), UT_ASCII)
  File "cf_units/_udunits2.pyx", line 268, in cf_units._udunits2.parse
    return wrap_unit(system, cunit)
  File "cf_units/_udunits2.pyx", line 123, in cf_units._udunits2.wrap_unit
    _raise_error()
  File "cf_units/_udunits2.pyx", line 196, in cf_units._udunits2._raise_error
    raise UdunitsError(status, errnum)
cf_units._udunits2.UdunitsError: UT_SUCCESS

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/pelson/dev/scitools/cf-units/cf_units/__init__.py", line 836, in __init__
    self._propogate_error('Failed to parse unit "%s"' % unit, e)
  File "/Users/pelson/dev/scitools/cf-units/cf_units/__init__.py", line 871, in _propogate_error
    raise ValueError('[%s] %s%s' % (ud_err.status_msg(), msg, error_msg))
ValueError: [UT_SUCCESS] Failed to parse unit "2^m"

With this change we now get good exceptions in both python2 and python3:

python2 -c "import cf_units as u; u.Unit('2^m')"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "cf_units/__init__.py", line 853, in __init__
    six.raise_from(value_error, None)
  File "/Users/pelson/dev/scitools/cf-units/env_py27/lib/python2.7/site-packages/six.py", line 737, in raise_from
    raise value
ValueError: [UT_SUCCESS] Failed to parse unit "2^m"

$ python3 -c "import cf_units as u; u.Unit('2^m')"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/pelson/dev/scitools/cf-units/cf_units/__init__.py", line 853, in __init__
    six.raise_from(value_error, None)
  File "<string>", line 3, in raise_from
ValueError: [UT_SUCCESS] Failed to parse unit "2^m"

coveralls · 2019-01-21T17:13:47Z

Coverage increased (+3.4%) to 91.435% when pulling 683dde7 on pelson:raise_from into 659e994 on SciTools:master.

cf_units/__init__.py

cf_units/tests/test_unit.py

pelson · 2019-01-22T10:28:31Z

This PR now depends upon #135, as I added a few unicode tests on Unit.format in order to assure myself that the exception was not being raised in any situation.

bjlittle · 2019-01-22T14:53:07Z

@pelson Rebase after recent merges?

pelson · 2019-01-22T16:18:49Z

@bjlittle, let's get #137 in first, then I'll rebase this.

…we don't have multiple tracebacks in python 3. Also improved coverage of Unit.log, which was highlighted by this change. Bolster the unit tests to ensure that some of the exception handling is redundant.

pelson · 2019-01-24T10:54:10Z

OK, now rebased @bjlittle. Take it away.

pelson · 2019-01-24T11:23:43Z

cf_units/tests/test_unit.py

@@ -189,6 +190,18 @@ def test_format_unit_definition(self):
        u = Unit('watt')
        self.assertEqual(u.format(unit.UT_DEFINITION), 'm2.kg.s-3')

+    def test_format_multiple_options(self):


I don't think this was tested anywhere before!

pelson · 2019-01-24T11:24:06Z

cf_units/tests/test_unit.py

+        u = Unit('watt')
+        self.assertEqual(
+            u.format([unit.UT_NAMES, unit.UT_DEFINITION, unit.UT_UTF8]),
+            u'meter²·kilogram·second⁻³')


Pretty cool eh?

pelson · 2019-01-24T11:25:04Z

cf_units/tests/test_unit.py

+
+    def test_format_latin1(self):
+        u = Unit('m2')
+        self.assertEqual(u.format(unit.UT_LATIN1), u'm²')


From a coverage perspective, LATIN1 == ISO8859-1

pelson · 2019-01-24T11:25:19Z

cf_units/tests/test_unit.py

+    def test_negative(self):
+        u = Unit('hPa')
+        msg = re.escape("Failed to calculate logorithmic base of Unit('hPa')")
+        with six.assertRaisesRegex(self, ValueError, msg):


No previous test of this.

bjlittle · 2019-01-25T12:46:29Z

cf_units/__init__.py

-                    self._propogate_error('Failed to format %r' % self, e)
-            return str(result.decode(endocing_str))
+            encoding_str = _encoding_lookup[encoding]
+            result = _ud.format(self.ut_unit, bitmask)


Discussed with @pelson, removed unnecessary defensive try...except.

There were no test cases that raised this exception for _ud.format.

bjlittle · 2019-01-25T12:46:58Z

cf_units/__init__.py

-            except _ud.UdunitsError as e:
-                self._propogate_error('Failed to invert %r' % self, e)
-            calendar = None
+            ut_unit = _ud.invert(self.ut_unit)


Discussed with @pelson, removed unnecessary defensive try...except.

There were no test cases that raised this exception for _ud.invert.

bjlittle · 2019-01-25T12:49:02Z

Lovely, thanks @pelson 👍

pelson requested a review from bjlittle January 21, 2019 17:00

pelson added the Type: Enhancement label Jan 21, 2019

pelson force-pushed the raise_from branch from f03553a to f54f288 Compare January 21, 2019 17:10

stickler-ci reviewed Jan 21, 2019

View reviewed changes

cf_units/__init__.py Outdated Show resolved Hide resolved

cf_units/__init__.py Outdated Show resolved Hide resolved

cf_units/__init__.py Outdated Show resolved Hide resolved

stickler-ci reviewed Jan 22, 2019

View reviewed changes

cf_units/tests/test_unit.py Outdated Show resolved Hide resolved

pelson force-pushed the raise_from branch from c3b6b32 to 73b8195 Compare January 22, 2019 10:38

bjlittle self-assigned this Jan 22, 2019

pelson force-pushed the raise_from branch from 73b8195 to c7cad47 Compare January 24, 2019 10:52

Tidy up the propogation of exceptions from UDUNITS2, and ensure that …

683dde7

…we don't have multiple tracebacks in python 3. Also improved coverage of Unit.log, which was highlighted by this change. Bolster the unit tests to ensure that some of the exception handling is redundant.

pelson force-pushed the raise_from branch from c7cad47 to 683dde7 Compare January 24, 2019 10:53

pelson commented Jan 24, 2019

View reviewed changes

bjlittle reviewed Jan 25, 2019

View reviewed changes

bjlittle merged commit 6dbb917 into SciTools:master Jan 25, 2019

stephenworsley mentioned this pull request Jun 22, 2020

Python segmentation violation with invalid unit and pytest #160

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tidy up the propagation of exceptions from UDUNITS2 #136

Tidy up the propagation of exceptions from UDUNITS2 #136

pelson commented Jan 21, 2019

coveralls commented Jan 21, 2019 •

edited

Loading

pelson commented Jan 22, 2019

bjlittle commented Jan 22, 2019

pelson commented Jan 22, 2019

pelson commented Jan 24, 2019

pelson Jan 24, 2019

pelson Jan 24, 2019

pelson Jan 24, 2019

pelson Jan 24, 2019

bjlittle Jan 25, 2019

bjlittle Jan 25, 2019

bjlittle commented Jan 25, 2019

Tidy up the propagation of exceptions from UDUNITS2 #136

Tidy up the propagation of exceptions from UDUNITS2 #136

Conversation

pelson commented Jan 21, 2019

coveralls commented Jan 21, 2019 • edited Loading

pelson commented Jan 22, 2019

bjlittle commented Jan 22, 2019

pelson commented Jan 22, 2019

pelson commented Jan 24, 2019

pelson Jan 24, 2019

Choose a reason for hiding this comment

pelson Jan 24, 2019

Choose a reason for hiding this comment

pelson Jan 24, 2019

Choose a reason for hiding this comment

pelson Jan 24, 2019

Choose a reason for hiding this comment

bjlittle Jan 25, 2019

Choose a reason for hiding this comment

bjlittle Jan 25, 2019

Choose a reason for hiding this comment

bjlittle commented Jan 25, 2019

coveralls commented Jan 21, 2019 •

edited

Loading