Repository: Qiskit/qiskit-optimization
Pull request #182 (2021-06-28 16:58:40)

Move away from using pylint header plugin & new deprecate_values decorator

<!--
⚠️ If you do not respect this template, your pull request will be closed.
⚠️ Your pull request title should be short detailed and understandable for all.
⚠️ Also, please add it in the CHANGELOG file under Unreleased section.
⚠️ If your pull request fixes an open issue, please link to the issue.

✅ I have added the tests to cover my changes.
✅ I have updated the documentation accordingly.
✅ I have read the CONTRIBUTING document.
-->

### Summary
Equivalent to Qiskit/qiskit-nature#251


### Details and comments





Patch:
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index ef4dd7f..ee8aab9 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -85,6 +85,7 @@ jobs:
   Lint:
     runs-on: ubuntu-latest
     strategy:
+      fail-fast: false
       matrix:
         python-version: [3.6, 3.7, 3.8, 3.9]
     steps:
diff --git a/.pylintdict b/.pylintdict
index f74c6c1..9bcc8b2 100644
--- a/.pylintdict
+++ b/.pylintdict
@@ -132,6 +132,8 @@ preprint
 presolver
 princeton
 probabilistically
+py
+pxd
 qaoa
 qasm
 qiskit
diff --git a/.pylintrc b/.pylintrc
index a13ebe6..09dbd41 100644
--- a/.pylintrc
+++ b/.pylintrc
@@ -23,9 +23,6 @@ persistent=yes
 # usually to register additional checkers.
 load-plugins=pylint.extensions.docparams, # enable checking of docstring args
     pylint.extensions.docstyle,  # basic docstring style checks
-    pylintfileheader  # Check license comments
-
-file-header=(?:(?:#[^\n]*)?\n)*# This code is part of Qiskit.\n#\n# \(C\) Copyright IBM [0-9, -]*.\n#\n# This code is licensed under the Apache License, Version 2.0. You may\n# obtain a copy of this license in the LICENSE.txt file in the root directory\n# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0.\n#\n# Any modifications or derivative works of this code must retain this\n# copyright notice, and modified files need to carry a notice indicating\n# that they have been altered from the originals.\n
 
 # Use multiple processes to speed up Pylint.
 jobs=1
diff --git a/Makefile b/Makefile
index 9a83203..c6daa68 100644
--- a/Makefile
+++ b/Makefile
@@ -41,6 +41,7 @@ all_check: spell style lint copyright mypy clean_sphinx html doctest
 
 lint:
 	pylint -rn qiskit_optimization test tools
+	python tools/verify_headers.py qiskit_optimization test tools
 
 mypy:
 	mypy qiskit_optimization test tools
diff --git a/qiskit_optimization/deprecation.py b/qiskit_optimization/deprecation.py
index 3d068b4..520641d 100644
--- a/qiskit_optimization/deprecation.py
+++ b/qiskit_optimization/deprecation.py
@@ -15,7 +15,8 @@
 from abc import abstractmethod
 import warnings
 import functools
-from typing import NamedTuple, Optional, Callable, Dict, Set
+import inspect
+from typing import NamedTuple, Optional, Callable, Dict, Set, cast, Any
 from enum import Enum, EnumMeta
 
 
@@ -76,7 +77,7 @@ class DeprecatedType(Enum):
     PROPERTY = "property"
 
 
-class _DeprecatedObject(NamedTuple):
+class _DeprecatedTypeName(NamedTuple):
     version: str
     old_type: DeprecatedType
     old_name: str
@@ -85,7 +86,24 @@ class _DeprecatedObject(NamedTuple):
     additional_msg: str
 
 
-_DEPRECATED_OBJECTS: Set[_DeprecatedObject] = set()
+class _DeprecatedArgument(NamedTuple):
+    version: str
+    func_qualname: str
+    old_arg: str
+    new_arg: str
+    additional_msg: str
+
+
+class _DeprecatedValue(NamedTuple):
+    version: str
+    func_qualname: str
+    argument: str
+    old_value: str
+    new_value: str
+    additional_msg: str
+
+
+_DEPRECATED_OBJECTS: Set[NamedTuple] = set()
 
 
 def warn_deprecated(
@@ -108,11 +126,11 @@ def warn_deprecated(
         stack_level: stack level
     """
     # skip if it was already added
-    obj = _DeprecatedObject(version, old_type, old_name, new_type, new_name, additional_msg)
+    obj = _DeprecatedTypeName(version, old_type, old_name, new_type, new_name, additional_msg)
     if obj in _DEPRECATED_OBJECTS:
         return
 
-    _DEPRECATED_OBJECTS.add(obj)
+    _DEPRECATED_OBJECTS.add(cast(NamedTuple, obj))
 
     msg = (
         f"The {old_name} {old_type.value} is deprecated as of version {version} "
@@ -149,7 +167,7 @@ def warn_deprecated_same_type_name(
     )
 
 
-def _rename_kwargs(version, func_name, kwargs, kwarg_map, additional_msg, stack_level):
+def _rename_kwargs(version, qualname, func_name, kwargs, kwarg_map, additional_msg, stack_level):
     for old_arg, new_arg in kwarg_map.items():
         if old_arg in kwargs:
             if new_arg in kwargs:
@@ -157,29 +175,103 @@ def _rename_kwargs(version, func_name, kwargs, kwarg_map, additional_msg, stack_
                     "{} received both {} and {} (deprecated).".format(func_name, new_arg, old_arg)
                 )
 
+            # skip if it was already added
+            obj = _DeprecatedArgument(version, qualname, old_arg, new_arg, additional_msg)
+            if obj not in _DEPRECATED_OBJECTS:
+                _DEPRECATED_OBJECTS.add(obj)
+
+                msg = (
+                    f"{func_name}: the {old_arg} {DeprecatedType.ARGUMENT.value} is deprecated "
+                    f"as of version {version} and will be removed no sooner "
+                    "than 3 months after the release. Instead use the "
+                    f"{new_arg} {DeprecatedType.ARGUMENT.value}"
+                )
+                if additional_msg:
+                    msg += f" {additional_msg}"
+                msg += "."
+                warnings.warn(msg, DeprecationWarning, stacklevel=stack_level)
+            kwargs[new_arg] = kwargs.pop(old_arg)
+
+
+def deprecate_arguments(
+    version: str,
+    kwarg_map: Dict[str, str],
+    additional_msg: Optional[str] = None,
+    stack_level: int = 3,
+) -> Callable:
+    """Decorator to alias deprecated argument names and warn upon use.
+    Args:
+        version: Version to be used
+        kwarg_map: Args dictionary with old, new arguments
+        additional_msg: any additional message
+        stack_level: stack level
+
+    Returns:
+        The decorated function
+    """
+
+    def decorator(func):
+        @functools.wraps(func)
+        def wrapper(*args, **kwargs):
+            if kwargs:
+                _rename_kwargs(
+                    version,
+                    func.__qualname__,
+                    func.__name__,
+                    kwargs,
+                    kwarg_map,
+                    additional_msg,
+                    stack_level,
+                )
+            return func(*args, **kwargs)
+
+        return wrapper
+
+    return decorator
+
+
+def _check_values(version, qualname, args, kwargs, kwarg_map, additional_msg, stack_level):
+    for arg, value_dict in kwarg_map.items():
+        index = value_dict["index"]
+        values_map = value_dict["values"]
+        old_value = None
+        if args and 0 < index < len(args):
+            old_value = args[index]
+        if kwargs and arg in kwargs:
+            old_value = kwargs[arg]
+
+        if old_value in values_map:
+            new_value = values_map[old_value]
+
+            # skip if it was already added
+            obj = _DeprecatedValue(version, qualname, arg, old_value, new_value, additional_msg)
+            if obj in _DEPRECATED_OBJECTS:
+                continue
+
+            _DEPRECATED_OBJECTS.add(obj)
+
             msg = (
-                f"{func_name}: the {old_arg} {DeprecatedType.ARGUMENT.value} is deprecated "
+                f'The {arg} {DeprecatedType.ARGUMENT.value} value "{old_value}" is deprecated '
                 f"as of version {version} and will be removed no sooner "
                 "than 3 months after the release. Instead use the "
-                f"{new_arg} {DeprecatedType.ARGUMENT.value}"
+                f'"{new_value}" value'
             )
             if additional_msg:
                 msg += f" {additional_msg}"
             msg += "."
             warnings.warn(msg, DeprecationWarning, stacklevel=stack_level)
-            kwargs[new_arg] = kwargs.pop(old_arg)
 
 
-def deprecate_arguments(
+def deprecate_values(
     version: str,
-    kwarg_map: Dict[str, str],
+    kwarg_map: Dict[str, Dict[Any, Any]],
     additional_msg: Optional[str] = None,
     stack_level: int = 3,
 ) -> Callable:
-    """Decorator to alias deprecated argument names and warn upon use.
+    """Decorator to alias deprecated default values and warn upon use.
     Args:
         version: Version to be used
-        kwarg_map: Args dictionary with old, new arguments
+        kwarg_map: Args dictionary with argument and map of old, new values
         additional_msg: any additional message
         stack_level: stack level
 
@@ -190,9 +282,22 @@ def deprecate_arguments(
     def decorator(func):
         @functools.wraps(func)
         def wrapper(*args, **kwargs):
-            if kwargs:
-                _rename_kwargs(
-                    version, func.__name__, kwargs, kwarg_map, additional_msg, stack_level
+            if args or kwargs:
+                parameter_names = list(inspect.signature(func).parameters.keys())
+                new_kwarg_map = {}
+                for name, values_map in kwarg_map.items():
+                    new_kwarg_map[name] = {
+                        "index": parameter_names.index(name),
+                        "values": values_map,
+                    }
+                _check_values(
+                    version,
+                    func.__qualname__,
+                    args,
+                    kwargs,
+                    new_kwarg_map,
+                    additional_msg,
+                    stack_level,
                 )
             return func(*args, **kwargs)
 
diff --git a/requirements-dev.txt b/requirements-dev.txt
index 73f139d..e4134f8 100644
--- a/requirements-dev.txt
+++ b/requirements-dev.txt
@@ -2,7 +2,6 @@ coverage>=4.4.0
 matplotlib>=2.1
 black==21.4b2
 pylint>=2.7.1
-pylintfileheader>=0.0.2
 pylatexenc>=1.4
 stestr>=2.0.0
 ddt>=1.2.0,!=1.4.0
diff --git a/tools/verify_headers.py b/tools/verify_headers.py
new file mode 100755
index 0000000..b77b53e
--- /dev/null
+++ b/tools/verify_headers.py
@@ -0,0 +1,110 @@
+#!/usr/bin/env python3
+# This code is part of Qiskit.
+#
+# (C) Copyright IBM 2020, 2021.
+#
+# This code is licensed under the Apache License, Version 2.0. You may
+# obtain a copy of this license in the LICENSE.txt file in the root directory
+# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0.
+#
+# Any modifications or derivative works of this code must retain this
+# copyright notice, and modified files need to carry a notice indicating
+# that they have been altered from the originals.
+
+"""Utility script to verify qiskit copyright file headers"""
+
+import argparse
+import multiprocessing
+import os
+import sys
+import re
+
+# regex for character encoding from PEP 263
+pep263 = re.compile(r"^[ \t\f]*#.*?coding[:=][ \t]*([-_.a-zA-Z0-9]+)")
+
+
+def discover_files(code_paths, exclude_folders):
+    """Find all .py, .pyx, .pxd files in a list of trees"""
+    out_paths = []
+    for path in code_paths:
+        if os.path.isfile(path):
+            out_paths.append(path)
+        else:
+            for directory in os.walk(path):
+                dir_path = directory[0]
+                for folder in exclude_folders:
+                    if folder in directory[1]:
+                        directory[1].remove(folder)
+                for subfile in directory[2]:
+                    if (
+                        subfile.endswith(".py")
+                        or subfile.endswith(".pyx")
+                        or subfile.endswith(".pxd")
+                    ):
+                        out_paths.append(os.path.join(dir_path, subfile))
+    return out_paths
+
+
+def validate_header(file_path):
+    """Validate the header for a single file"""
+    header = """# This code is part of Qiskit.
+#
+"""
+    apache_text = """#
+# This code is licensed under the Apache License, Version 2.0. You may
+# obtain a copy of this license in the LICENSE.txt file in the root directory
+# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0.
+#
+# Any modifications or derivative works of this code must retain this
+# copyright notice, and modified files need to carry a notice indicating
+# that they have been altered from the originals.
+"""
+    count = 0
+    with open(file_path, encoding="utf8") as code_file:
+        lines = code_file.readlines()
+    start = 0
+    for index, line in enumerate(lines):
+        count += 1
+        if count > 5:
+            return file_path, False, "Header not found in first 5 lines"
+        if count <= 2 and pep263.match(line):
+            return file_path, False, "Unnecessary encoding specification (PEP 263, 3120)"
+        if line == "# This code is part of Qiskit.\n":
+            start = index
+            break
+    if "".join(lines[start : start + 2]) != header:
+        return (file_path, False, "Header up to copyright line does not match: %s" % header)
+    if not lines[start + 2].startswith("# (C) Copyright IBM 20"):
+        return (file_path, False, "Header copyright line not found")
+    if "".join(lines[start + 3 : start + 11]) != apache_text:
+        return (file_path, False, "Header apache text string doesn't match:\n %s" % apache_text)
+    return (file_path, True, None)
+
+
+def _main():
+    default_path = os.path.join(
+        os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "qiskit_optimization"
+    )
+    parser = argparse.ArgumentParser(description="Check file headers.")
+    parser.add_argument(
+        "paths",
+        type=str,
+        nargs="*",
+        default=[default_path],
+        help="Paths to scan by default uses ../qiskit_optimization from the script",
+    )
+    args = parser.parse_args()
+    files = discover_files(args.paths, exclude_folders=[])
+    with multiprocessing.Pool() as pool:
+        res = pool.map(validate_header, files)
+    failed_files = [x for x in res if x[1] is False]
+    if len(failed_files) > 0:
+        for failed_file in failed_files:
+            sys.stderr.write("%s failed header check because:\n" % failed_file[0])
+            sys.stderr.write("%s\n\n" % failed_file[2])
+        sys.exit(1)
+    sys.exit(0)
+
+
+if __name__ == "__main__":
+    _main()
diff --git a/tox.ini b/tox.ini
index d007e8b..f1db060 100644
--- a/tox.ini
+++ b/tox.ini
@@ -25,6 +25,7 @@ commands =
   pylint -rn qiskit_optimization test tools
   mypy qiskit_optimization test tools
   python3 {toxinidir}/tools/check_copyright.py -path {toxinidir}
+  python3 {toxinidir}/tools/verify_headers.py qiskit_optimization test tools
 
 [testenv:black]
 envdir = .tox/lint


Test patch:
diff --git a/test/test_deprecation.py b/test/test_deprecation.py
index 9b6dc22..9a2b1c9 100644
--- a/test/test_deprecation.py
+++ b/test/test_deprecation.py
@@ -31,13 +31,14 @@ from qiskit_optimization.deprecation import (
     deprecate_property,
     deprecate_method,
     deprecate_arguments,
+    deprecate_values,
 )
 
 # pylint: disable=bad-docstring-quotes
 
 
 class EnumTest(DeprecatedEnum, metaclass=DeprecatedEnumMeta):
-    """Enum Test"""
+    """Enumeration Test"""
 
     ONE = "one"
     TWO = "two"
@@ -74,6 +75,7 @@ class DeprecatedClass1:
         warn_deprecated(
             "0.3.0", DeprecatedType.CLASS, "DeprecatedClass1", DeprecatedType.CLASS, "NewClass"
         )
+        self.value = 10
 
 
 class DeprecatedClass2:
@@ -83,6 +85,36 @@ class DeprecatedClass2:
         warn_deprecated_same_type_name(
             "0.3.0", DeprecatedType.CLASS, "DeprecatedClass2", "from package test2"
         )
+        self.value = 10
+
+
+class DeprecatedClass3:
+    """Deprecated Test class 3"""
+
+    @deprecate_values("0.2.0", {"loss": {"l1": "absolute_error", "l2": "squared_error"}})
+    def __init__(self, arg1: int, loss: str = "squared_error") -> None:
+        self.value = arg1
+        self.loss = loss
+
+    @deprecate_values("0.2.0", {"loss": {"l1": "absolute_error", "l2": "squared_error"}})
+    def method1(self, arg1: int, loss: str = "squared_error") -> None:
+        """method 1"""
+        self.value = arg1
+        self.loss = loss
+
+
+class DeprecatedClass4:
+    """Deprecated Test class 4"""
+
+    def __init__(
+        self,
+    ) -> None:
+        self.status = 2
+
+    @deprecate_values("0.2.0", {"status": {3: 2, 4: 5}})
+    def method1(self, status: int = 2) -> None:
+        """method 1"""
+        self.status = status
 
 
 class TestClass:
@@ -141,7 +173,7 @@ class TestDeprecation(QiskitOptimizationTestCase):
         return -1
 
     def test_enum_deprecation(self):
-        """test enum deprecation"""
+        """test enumeration deprecation"""
 
         msg_ref = (
             "The EnumTest enum is deprecated as of version 0.2.0 "
@@ -210,7 +242,8 @@ class TestDeprecation(QiskitOptimizationTestCase):
         # emit deprecation the first time it is used
         with warnings.catch_warnings(record=True) as c_m:
             warnings.simplefilter("always")
-            DeprecatedClass1()
+            obj = DeprecatedClass1()
+            self.assertEqual(obj.value, 10)
             msg = str(c_m[0].message)
             self.assertEqual(msg, msg_ref)
             self.assertTrue("test_deprecation.py" in c_m[0].filename, c_m[0].filename)
@@ -219,7 +252,8 @@ class TestDeprecation(QiskitOptimizationTestCase):
         # trying again should not emit deprecation
         with warnings.catch_warnings(record=True) as c_m:
             warnings.simplefilter("always")
-            DeprecatedClass1()
+            obj = DeprecatedClass1()
+            self.assertEqual(obj.value, 10)
             self.assertListEqual(c_m, [])
 
     def test_class_deprecation2(self):
@@ -234,7 +268,8 @@ class TestDeprecation(QiskitOptimizationTestCase):
         # emit deprecation the first time it is used
         with warnings.catch_warnings(record=True) as c_m:
             warnings.simplefilter("always")
-            DeprecatedClass2()
+            obj = DeprecatedClass2()
+            self.assertEqual(obj.value, 10)
             msg = str(c_m[0].message)
             self.assertEqual(msg, msg_ref)
             self.assertTrue("test_deprecation.py" in c_m[0].filename, c_m[0].filename)
@@ -243,7 +278,101 @@ class TestDeprecation(QiskitOptimizationTestCase):
         # trying again should not emit deprecation
         with warnings.catch_warnings(record=True) as c_m:
             warnings.simplefilter("always")
-            DeprecatedClass2()
+            obj = DeprecatedClass2()
+            self.assertEqual(obj.value, 10)
+            self.assertListEqual(c_m, [])
+
+    @data(
+        (
+            "l1",
+            'The loss argument value "l1" is deprecated as of version 0.2.0 '
+            "and will be removed no sooner than 3 months after the release. "
+            'Instead use the "absolute_error" value.',
+        ),
+        (
+            "l2",
+            'The loss argument value "l2" is deprecated as of version 0.2.0 '
+            "and will be removed no sooner than 3 months after the release. "
+            'Instead use the "squared_error" value.',
+        ),
+    )
+    def test_string_values_deprecation(self, config):
+        """test string values deprecation"""
+
+        loss, msg_ref = config
+
+        # emit deprecation the first time it is used
+        with warnings.catch_warnings(record=True) as c_m:
+            warnings.simplefilter("always")
+            obj = DeprecatedClass3(10, loss=loss)
+            self.assertEqual(obj.value, 10)
+            msg = str(c_m[0].message)
+            self.assertEqual(msg, msg_ref)
+            self.assertTrue("test_deprecation.py" in c_m[0].filename, c_m[0].filename)
+            self.assertEqual(
+                self._get_line_from_str("DeprecatedClass3(10, loss=loss)"), c_m[0].lineno
+            )
+
+        # trying again should not emit deprecation
+        with warnings.catch_warnings(record=True) as c_m:
+            warnings.simplefilter("always")
+            obj = DeprecatedClass3(10, loss=loss)
+            self.assertEqual(obj.value, 10)
+            self.assertListEqual(c_m, [])
+
+        # emit deprecation the first time it is used
+        with warnings.catch_warnings(record=True) as c_m:
+            warnings.simplefilter("always")
+            obj = DeprecatedClass3(5, loss=loss)
+            obj.method1(10, loss)
+            self.assertEqual(obj.value, 10)
+            msg = str(c_m[0].message)
+            self.assertEqual(msg, msg_ref)
+            self.assertTrue("test_deprecation.py" in c_m[0].filename, c_m[0].filename)
+            self.assertEqual(self._get_line_from_str("obj.method1(10, loss)"), c_m[0].lineno)
+
+        # trying again should not emit deprecation
+        with warnings.catch_warnings(record=True) as c_m:
+            warnings.simplefilter("always")
+            obj = DeprecatedClass3(5, loss=loss)
+            obj.method1(10, loss)
+            self.assertEqual(obj.value, 10)
+            self.assertListEqual(c_m, [])
+
+    @data(
+        (
+            3,
+            'The status argument value "3" is deprecated as of version 0.2.0 '
+            "and will be removed no sooner than 3 months after the release. "
+            'Instead use the "2" value.',
+        ),
+        (
+            4,
+            'The status argument value "4" is deprecated as of version 0.2.0 '
+            "and will be removed no sooner than 3 months after the release. "
+            'Instead use the "5" value.',
+        ),
+    )
+    def test_int_values_deprecation(self, config):
+        """test int values deprecation"""
+
+        status, msg_ref = config
+        obj = DeprecatedClass4()
+
+        # emit deprecation the first time it is used
+        with warnings.catch_warnings(record=True) as c_m:
+            warnings.simplefilter("always")
+            obj.method1(status)
+            msg = str(c_m[0].message)
+            self.assertEqual(msg, msg_ref)
+            self.assertTrue("test_deprecation.py" in c_m[0].filename, c_m[0].filename)
+            self.assertEqual(self._get_line_from_str("obj.method1(status)"), c_m[0].lineno)
+
+        # trying again should not emit deprecation
+        with warnings.catch_warnings(record=True) as c_m:
+            warnings.simplefilter("always")
+            obj.method1(status)
+            self.assertEqual(obj.status, status)
             self.assertListEqual(c_m, [])
 
     @data(
@@ -301,6 +430,12 @@ class TestDeprecation(QiskitOptimizationTestCase):
             self.assertTrue("test_deprecation.py" in c_m[0].filename, c_m[0].filename)
             self.assertEqual(self._get_line_from_str('func3(old_arg="hello")'), c_m[0].lineno)
 
+        # trying again should not emit deprecation
+        with warnings.catch_warnings(record=True) as c_m:
+            warnings.simplefilter("always")
+            self.assertEqual(("hello", None), func3(old_arg="hello"))
+            self.assertListEqual(c_m, [])
+
     def test_method_arguments_deprecation(self):
         """test method arguments deprecation"""
 
@@ -323,6 +458,12 @@ class TestDeprecation(QiskitOptimizationTestCase):
             self.assertTrue("test_deprecation.py" in c_m[0].filename, c_m[0].filename)
             self.assertEqual(self._get_line_from_str('obj.method3(old_arg="hello")'), c_m[0].lineno)
 
+        # trying again should not emit deprecation
+        with warnings.catch_warnings(record=True) as c_m:
+            warnings.simplefilter("always")
+            self.assertEqual(("hello", None), obj.method3(old_arg="hello"))
+            self.assertListEqual(c_m, [])
+
     def test_property_deprecation(self):
         """test property deprecation"""