From b8b40a28f2389264a2c1c159af294e1fcd5ba76e Mon Sep 17 00:00:00 2001
From: Hans Dembinski <HDembinski@users.noreply.github.com>
Date: Sun, 14 Jun 2020 14:10:07 +0200
Subject: [PATCH] bug fix for merrors getitem (#422)

---
 doc/reference.rst                    |  4 ++--
 src/iminuit/tests/test_deprecated.py |  8 +++++--
 src/iminuit/tests/test_iminuit.py    |  8 +++++++
 src/iminuit/util.py                  | 35 ++++++++++++++++++----------
 4 files changed, 39 insertions(+), 16 deletions(-)

diff --git a/doc/reference.rst b/doc/reference.rst
index 39f96f89b..408375506 100644
--- a/doc/reference.rst
+++ b/doc/reference.rst
@@ -36,7 +36,7 @@ Minuit
 .. autoclass:: Minuit
     :members:
     :undoc-members:
-    :exclude-members: is_fixed, print_all_minos, print_fmin, print_param, print_initial_param, print_matrix, set_errordef, set_strategy, set_print_level, set_up, from_array_func, get_merrors, list_of_vary_param, list_of_fixed_param, get_fmin, get_initial_param_states, get_param_states, get_num_call_fcn, get_num_call_grad, matrix_accurate, merrors, migrad_ok
+    :exclude-members: is_fixed, print_all_minos, print_fmin, print_param, print_initial_param, print_matrix, set_errordef, set_strategy, set_print_level, set_up, from_array_func, get_merrors, list_of_vary_param, list_of_fixed_param, get_fmin, get_initial_param_states, get_param_states, get_num_call_fcn, get_num_call_grad, matrix_accurate, migrad_ok, merrors_struct
 
     .. automethod:: from_array_func
 
@@ -128,7 +128,7 @@ Minos Data Object
 ~~~~~~~~~~~~~~~~~
 
 Subclass of NamedTuple which stores information about the Minos result. It is returned by :meth:`Minuit.minos`
-(as part of a dictionary from parameter name -> data object). You can get it also from :meth:`Minuit.get_merrors`. It has the following attributes:
+(as part of a dictionary from parameter name -> data object). You can get it also from :meth:`Minuit.merrors`. It has the following attributes:
 
     * *lower*: lower error value
 
diff --git a/src/iminuit/tests/test_deprecated.py b/src/iminuit/tests/test_deprecated.py
index 3e720daa4..46ce95db7 100644
--- a/src/iminuit/tests/test_deprecated.py
+++ b/src/iminuit/tests/test_deprecated.py
@@ -21,10 +21,14 @@ def minuit():
 
 def test_minos_merrors(minuit):
     minuit.minos()
+    m = minuit.merrors
     with pytest.warns(DeprecationWarning):
-        assert minuit.merrors[("x", -1.0)] == approx(-1)
+        assert m[("x", -1)] == approx(-1)
     with pytest.warns(DeprecationWarning):
-        assert minuit.merrors[("x", 1.0)] == approx(1)
+        assert m[("x", 1)] == approx(1)
+    with pytest.warns(DeprecationWarning):
+        with pytest.raises(ValueError):
+            m[("x", 2)]
 
 
 def test_default_errordef():
diff --git a/src/iminuit/tests/test_iminuit.py b/src/iminuit/tests/test_iminuit.py
index 576e5cac7..9bf8dbc85 100644
--- a/src/iminuit/tests/test_iminuit.py
+++ b/src/iminuit/tests/test_iminuit.py
@@ -483,6 +483,14 @@ def test_minos_all(grad, sigma):
     assert m.merrors["y"].upper == approx(sigma * 1)
     assert m.merrors[0].lower == approx(-sigma * 2)
     assert m.merrors[1].upper == approx(sigma * 1)
+    assert m.merrors[-1].upper == approx(sigma * 1)
+
+    with pytest.raises(KeyError):
+        m.merrors["xy"]
+    with pytest.raises(KeyError):
+        m.merrors["z"]
+    with pytest.raises(IndexError):
+        m.merrors[-3]
 
 
 @pytest.mark.parametrize("grad", (None, func0_grad))
diff --git a/src/iminuit/util.py b/src/iminuit/util.py
index 33954f606..d35aba46c 100644
--- a/src/iminuit/util.py
+++ b/src/iminuit/util.py
@@ -227,26 +227,37 @@ def _repr_pretty_(self, p, cycle):
 
     def __getitem__(self, key):
         is_deprecated_call = False
-        try:
-            key, ul = key
-            ul = int(ul)
-            is_deprecated_call = True
-            warnings.warn(
-                "`merrors[(name, +-1)]` is deprecated; use `merrors[name]`",
-                DeprecationWarning,
-                stacklevel=2,
-            )
-        except (ValueError, TypeError):
-            pass
-
         if isinstance(key, int):
+            if key >= len(self):
+                raise IndexError("index out of range")
+            if key < 0:
+                key += len(self)
+            if key < 0:
+                raise IndexError("index out of range")
             for k in self.keys():
                 if key == 0:
                     key = k
                     break
                 key -= 1
+        else:
+            if not isinstance(key, str):
+                try:
+                    k, ul = key
+                    ul = int(ul)
+                    is_deprecated_call = True
+                    key = k
+                    warnings.warn(
+                        "`merrors[(name, +-1)]` is deprecated; use `merrors[name]`",
+                        DeprecationWarning,
+                        stacklevel=2,
+                    )
+                except (ValueError, TypeError):
+                    pass
+
         v = super(MErrors, self).__getitem__(key)
         if is_deprecated_call:
+            if ul not in (-1, 1):
+                raise ValueError("key must be (name, 1) or (name, -1)")
             return v.lower if ul == -1 else v.upper
         return v