Repository: snowblink14/smatch

Issue:
Remove Python 2 support
Python 2 was retired at the beginning of the year, and it doesn't even ship with the lastest Ubuntu LTS (20.04) by default. I think smatch should remove explicit support for Python 2. Mainly this means that any workarounds for Python 2 are removed and it is no longer listed in `setup.py`, `tox.ini`, and `.travis.yml` as a supported version. Users who absolutely need Python 2 should pin their smatch version to the current or previous version.

I've already prepared a branch for a pull request, but I created this issue in case there is any discussion about what to do.

Discussion:

Patch:
diff --git a/.appveyor.yml b/.appveyor.yml
index 48cc9c0..1c1b163 100644
--- a/.appveyor.yml
+++ b/.appveyor.yml
@@ -3,6 +3,7 @@ environment:
   - TOXENV: py35
   - TOXENV: py36
   - TOXENV: py37
+  - TOXENV: py38
 
 build: off
 
diff --git a/.travis.yml b/.travis.yml
index 50980a8..fbce818 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -3,13 +3,15 @@ python:
 - '3.5'
 - '3.6'
 - '3.7'
+- '3.8'
 install: pip install tox-travis
 script: tox
 deploy:
   provider: pypi
   user: __token__
   password:
-    secure: CVA8RIEx1IdOHUBMjF/MPd6FFE3wu0sAzJkowu0PzZ4VwbOlqSYXuYWjkNUV8plCVm8mgovMXBDjTC8q9AGYTJi8B5f92AY6YRfLVjJCpdMd8EH6VNlymxJTYg0t5W5RKpzdAyOLj5GyAPhVqY805TIE+ao2XKQ+UwlwRUq/SUhxi6gcvLsamuabfKg2OZxijKp6dPk+tqw33K+DYJVV3WOUqyI169Z3iXRhljghmLWiV6xdIs1/V34XsbdgRJDRs/kOsrhE19khOIUqZ/+to++qRFwLpgAsF71n23vGs/FwvP+ab1oMYtUNC8DHI2gmiGHO/ipE/FXqsYdYuhULnzs7nfx04YQoaZliD7Hbvze0zEczQZxHQpuHbykNND2WU20NOnckBzDgqhqzPIVjHwQTGFKMZqg7nZ8w81NrGSeGTXagtoBPtq6/RPU1alppFE4EU7/fZrAN/fXgiemcxmsK5Nl9Ps6zWoB2eFhCaLDeUI9BktIrp5nLplLQJmLfF+RfLaIYULD5hdwFzgcUTjG/nPr41XNMvwYvsM+rCGxlxBtL2Gc2xf7Kfv9T6vUr0eb0Yxp+oKNv7XN3j1rZ+0PIqMbi1dpMJu7OqxIR5lQB6bYwDcCeKxrFpnP/9drtXzjqGZnzCt/rdGnMUd9PrkZTHFXOrsq0ZtmFSNK5/m4=
+    secure: jXgeZ3fjIODIltATnt487VOBf6TIOp9bpd79r/wUG373pa6GVrWKEbX1LBAprmSW7dI+pYccQpeNF5OIsUJFFNjy59iLlKtfY+9N8ugD7rvuQL4pP+jwiVvuKB9SLyLwzJWrArsmVOju9JQUogtKXPJiflY1PE4fGV/Zjx+FNl6816HO8i2uHaLOBzDbNXtOCnAmioaM72cQ/qvtbKdkxF7u7xtAwqA8TEBeaf6PYR2b1mn8Vcgo7pG/uJGZtICPVINxv914jDpu0CDcWl3rmtVM1Q44XxKLV3c+6QUeonTy5s+35plDiWnI0F0HJiGfS51NYvUUaZZQKuNbIKg27dJcfhuzZDarD4C/P6fjN8NSdmB/B4P058pBkUTmmaeaLBDENBOlJAwRwHupRvsnWZ9xazw1edEVo2tnFPhAZJ+BBDv9lISiNg2kiQx6+5/BbnecjS4lyz3Kje+QljocDXHwshsSl8FxnmJ6TWI21yJJauKM4LGxHGyCrz0W1AsYw+S/cwppC1/BzS9ZLW5DdUeVzULGKzY1MqZJ7ABt70MLBMmrV5VjpUjtTS9dqVBMSvCxshR2hOM47x4wEzG9yQec4aDvHImEPOMXKHkzk6fVhm5qO9IxJTPldYrLcR/UFpcFdHKelIReqTQsjzaEI7CMCicux/DDpRt9msrCeQY=
   on:
     tags: true
   skip_existing: true
+  skip_cleanup: true
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 96c99a1..36492a2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,10 +3,15 @@
 
 ## Unreleased
 
+* Python 2 support is removed ([#31])
+* Python 3.8 support is added
 * Smatch version is taken from git tag (see [#22][])
 * Add CI/CD configuration (see PR [#23][])
 * Better handle deinversion of special roles ([#10][])
 * Get `smatch-table.py` working again (part of PR [#27][])
+* Add `tests/` subdirectory and `test_top.py` ([#25])
+* Make TOP relation's value a constant string and not the top node's
+  concept to avoid double-penalizing different top concepts ([#25])
 
 ## [1.0.2][]
 
@@ -166,5 +171,7 @@ The following are taken from an old `update_log` file:
 [#19]: https://github.com/snowblink14/smatch/issues/19
 [#22]: https://github.com/snowblink14/smatch/issues/22
 [#23]: https://github.com/snowblink14/smatch/pull/23
+[#25]: https://github.com/snowblink14/smatch/pull/25
 [#27]: https://github.com/snowblink14/smatch/pull/27
+[#31]: https://github.com/snowblink14/smatch/issues/31
 
diff --git a/amr.py b/amr.py
index 020f52e..182b6b5 100755
--- a/amr.py
+++ b/amr.py
@@ -420,8 +420,8 @@ class AMR(object):
             # each node has a relation list and attribute list
             relation_list.append(node_rel_list)
             attribute_list.append(node_attr_list)
-        # add TOP as an attribute. The attribute value is the top node value
-        attribute_list[0].append(["TOP", node_value_list[0]])
+        # add TOP as an attribute. The attribute value just needs to be constant
+        attribute_list[0].append(["TOP", 'top'])
         result_amr = AMR(node_name_list, node_value_list, relation_list, attribute_list)
         return result_amr
 
diff --git a/setup.py b/setup.py
index 42071d8..533921b 100644
--- a/setup.py
+++ b/setup.py
@@ -21,7 +21,7 @@ setup(name="smatch",
       url="https://github.com/snowblink14/smatch",
       license="MIT",
       classifiers=[
-          'Development Status :: 5 - Production/Stable'
+          'Development Status :: 5 - Production/Stable',
           'Environment :: Console',
           'Intended Audience :: Developers',
           'Intended Audience :: Information Technology',
@@ -31,6 +31,7 @@ setup(name="smatch",
           'Programming Language :: Python :: 3.5',
           'Programming Language :: Python :: 3.6',
           'Programming Language :: Python :: 3.7',
+          'Programming Language :: Python :: 3.8',
           'Topic :: Scientific/Engineering :: Information Analysis',
           'Topic :: Software Development :: Libraries :: Python Modules',
           'Topic :: Text Processing :: Linguistic',
diff --git a/smatch-table.py b/smatch-table.py
index 9e7f450..5c3ea83 100755
--- a/smatch-table.py
+++ b/smatch-table.py
@@ -1,7 +1,5 @@
 #!/usr/bin/env python
 
-from __future__ import print_function
-
 import sys
 import os
 import time
@@ -9,7 +7,6 @@ import time
 import amr
 import smatch
 
-
 ERROR_LOG = sys.stderr
 
 DEBUG_LOG = sys.stderr
@@ -151,21 +148,6 @@ def pprint_table(table):
         print("\n")
 
 
-def build_arg_parser():
-    """
-    Build an argument parser using argparse. Use it when python version is 2.7 or later.
-
-    """
-    parser = argparse.ArgumentParser(description="Smatch table calculator -- arguments")
-    parser.add_argument("--fl", type=argparse.FileType('r'), help='AMR ID list file')
-    parser.add_argument('-f', nargs='+', help='AMR IDs (at least one)')
-    parser.add_argument("-p", nargs='*', help="User list (can be none)")
-    parser.add_argument("--fd", default=isi_dir_pre, help="AMR File directory. Default=location on isi machine")
-    parser.add_argument('-r', type=int, default=4, help='Restart number (Default:4)')
-    parser.add_argument('-v', action='store_true', help='Verbose output (Default:False)')
-    return parser
-
-
 def cb(option, value, parser):
     """
     Callback function to handle variable number of arguments in optparse
@@ -262,9 +244,9 @@ def main(arguments):
         table[i+1].append(names[i])
         for j in range(0, len_name):
             if i != j:
-                start = time.time()
+                start = time.perf_counter()
                 table[i+1].append(compute_files(names[i], names[j], ids, args.fd, args.r))
-                end = time.time()
+                end = time.perf_counter()
                 if table[i+1][-1] != -1.0:
                     acc_time += end-start
             else:
@@ -283,22 +265,50 @@ def main(arguments):
 
 
 if __name__ == "__main__":
-    whole_start = time.time()
-    parser = None
-    args = None
+    whole_start = time.perf_counter()
+
     import argparse
-    parser = build_arg_parser()
+
+    parser = argparse.ArgumentParser(description="Smatch table calculator")
+    parser.add_argument(
+        "--fl",
+        type=argparse.FileType('r'),
+        help='AMR ID list file')
+    parser.add_argument(
+        '-f',
+        nargs='+',
+        help='AMR IDs (at least one)')
+    parser.add_argument(
+        "-p",
+        nargs='*',
+        help="User list (can be none)")
+    parser.add_argument(
+        "--fd",
+        default=isi_dir_pre,
+        help="AMR File directory. Default=location on isi machine")
+    parser.add_argument(
+        '-r',
+        type=int,
+        default=4,
+        help='Restart number (Default:4)')
+    parser.add_argument(
+        '-v',
+        action='store_true',
+        help='Verbose output (Default:False)')
+
     args = parser.parse_args()
+
     # Regularize fd, add "/" at the end if needed
     if args.fd[-1] != "/":
         args.fd += "/"
+
     # acc_time is the smatch calculation time
     acc_time = main(args)
-    whole_end = time.time()
+    whole_end = time.perf_counter()
     # time of the whole running process
     whole_time = whole_end - whole_start
+
     # print if needed
     # print("Accumulated computation time", acc_time, file=ERROR_LOG)
     # print("Total time", whole_time, file=ERROR_LOG)
     # print("Percentage", float(acc_time)/float(whole_time), file=ERROR_LOG)
-
diff --git a/smatch.py b/smatch.py
index bd8d982..38f4f82 100755
--- a/smatch.py
+++ b/smatch.py
@@ -8,12 +8,9 @@ For detailed description of smatch, see http://www.isi.edu/natural-language/amr/
 
 """
 
-from __future__ import print_function
-from __future__ import division
+import random
 
 import amr
-import os
-import random
 import sys
 
 # total number of iteration in smatch computation
@@ -44,64 +41,6 @@ DEBUG_LOG = sys.stderr
 match_triple_dict = {}
 
 
-def build_arg_parser():
-    """
-    Build an argument parser using argparse. Use it when python version is 2.7 or later.
-
-    """
-    parser = argparse.ArgumentParser(description="Smatch calculator -- arguments")
-    parser.add_argument('-f', nargs=2, required=True, type=argparse.FileType('r'),
-                        help='Two files containing AMR pairs. AMRs in each file are separated by a single blank line')
-    parser.add_argument('-r', type=int, default=4, help='Restart number (Default:4)')
-    parser.add_argument('--significant', type=int, default=2, help='significant digits to output (default: 2)')
-    parser.add_argument('-v', action='store_true', help='Verbose output (Default:false)')
-    parser.add_argument('--vv', action='store_true', help='Very Verbose output (Default:false)')
-    parser.add_argument('--ms', action='store_true', default=False,
-                        help='Output multiple scores (one AMR pair a score)'
-                             'instead of a single document-level smatch score (Default: false)')
-    parser.add_argument('--pr', action='store_true', default=False,
-                        help="Output precision and recall as well as the f-score. Default: false")
-    parser.add_argument('--justinstance', action='store_true', default=False,
-                        help="just pay attention to matching instances")
-    parser.add_argument('--justattribute', action='store_true', default=False,
-                        help="just pay attention to matching attributes")
-    parser.add_argument('--justrelation', action='store_true', default=False,
-                        help="just pay attention to matching relations")
-
-    return parser
-
-
-def build_arg_parser2():
-    """
-    Build an argument parser using optparse. Use it when python version is 2.5 or 2.6.
-
-    """
-    usage_str = "Smatch calculator -- arguments"
-    parser = optparse.OptionParser(usage=usage_str)
-    parser.add_option("-f", "--files", nargs=2, dest="f", type="string",
-                      help='Two files containing AMR pairs. AMRs in each file are ' \
-                           'separated by a single blank line. This option is required.')
-    parser.add_option("-r", "--restart", dest="r", type="int", help='Restart number (Default: 4)')
-    parser.add_option('--significant', dest="significant", type="int", default=2,
-                      help='significant digits to output (default: 2)')
-    parser.add_option("-v", "--verbose", action='store_true', dest="v", help='Verbose output (Default:False)')
-    parser.add_option("--vv", "--veryverbose", action='store_true', dest="vv",
-                      help='Very Verbose output (Default:False)')
-    parser.add_option("--ms", "--multiple_score", action='store_true', dest="ms",
-                      help='Output multiple scores (one AMR pair a score) instead of ' \
-                           'a single document-level smatch score (Default: False)')
-    parser.add_option('--pr', "--precision_recall", action='store_true', dest="pr",
-                      help="Output precision and recall as well as the f-score. Default: false")
-    parser.add_option('--justinstance', action='store_true', default=False,
-                      help="just pay attention to matching instances")
-    parser.add_option('--justattribute', action='store_true', default=False,
-                      help="just pay attention to matching attributes")
-    parser.add_option('--justrelation', action='store_true', default=False,
-                      help="just pay attention to matching relations")
-    parser.set_defaults(r=4, v=False, ms=False, pr=False)
-    return parser
-
-
 def get_best_match(instance1, attribute1, relation1,
                    instance2, attribute2, relation2,
                    prefix1, prefix2, doinstance=True, doattribute=True, dorelation=True):
@@ -854,36 +793,62 @@ def main(arguments):
 
 
 if __name__ == "__main__":
-    parser = None
-    args = None
-    # use optparse if python version is 2.5 or 2.6
-    if sys.version_info[0] == 2 and sys.version_info[1] < 7:
-        import optparse
-
-        if len(sys.argv) == 1:
-            print("No argument given. Please run smatch.py -h to see the argument description.", file=ERROR_LOG)
-            exit(1)
-        parser = build_arg_parser2()
-        (args, opts) = parser.parse_args()
-        file_handle = []
-        if args.f is None:
-            print("smatch.py requires -f option to indicate two files \
-                                             containing AMR as input. Please run smatch.py -h to  \
-                                             see the argument description.", file=ERROR_LOG)
-            exit(1)
-        # assert there are 2 file names following -f.
-        assert (len(args.f) == 2)
-        for file_path in args.f:
-            if not os.path.exists(file_path):
-                print("Given file", args.f[0], "does not exist", file=ERROR_LOG)
-                exit(1)
-            file_handle.append(open(file_path, encoding='utf8'))
-        # use opened files
-        args.f = tuple(file_handle)
-    #  use argparse if python version is 2.7 or later
-    else:
-        import argparse
-
-        parser = build_arg_parser()
-        args = parser.parse_args()
+    import argparse
+
+    parser = argparse.ArgumentParser(description="Smatch calculator")
+    parser.add_argument(
+        '-f',
+        nargs=2,
+        required=True,
+        type=argparse.FileType('r'),
+        help=('Two files containing AMR pairs. '
+              'AMRs in each file are separated by a single blank line'))
+    parser.add_argument(
+        '-r',
+        type=int,
+        default=4,
+        help='Restart number (Default:4)')
+    parser.add_argument(
+        '--significant',
+        type=int,
+        default=2,
+        help='significant digits to output (default: 2)')
+    parser.add_argument(
+        '-v',
+        action='store_true',
+        help='Verbose output (Default:false)')
+    parser.add_argument(
+        '--vv',
+        action='store_true',
+        help='Very Verbose output (Default:false)')
+    parser.add_argument(
+        '--ms',
+        action='store_true',
+        default=False,
+        help=('Output multiple scores (one AMR pair a score) '
+              'instead of a single document-level smatch score '
+              '(Default: false)'))
+    parser.add_argument(
+        '--pr',
+        action='store_true',
+        default=False,
+        help=('Output precision and recall as well as the f-score. '
+              'Default: false'))
+    parser.add_argument(
+        '--justinstance',
+        action='store_true',
+        default=False,
+        help="just pay attention to matching instances")
+    parser.add_argument(
+        '--justattribute',
+        action='store_true',
+        default=False,
+        help="just pay attention to matching attributes")
+    parser.add_argument(
+        '--justrelation',
+        action='store_true',
+        default=False,
+        help="just pay attention to matching relations")
+
+    args = parser.parse_args()
     main(args)
diff --git a/tox.ini b/tox.ini
index 04ffd52..9a21d07 100644
--- a/tox.ini
+++ b/tox.ini
@@ -1,5 +1,5 @@
 [tox]
-envlist = py{35,36,37}
+envlist = py{35,36,37,38}
 
 [testenv]
 commands = python smatch.py -f test_input1.txt test_input2.txt


Test patch:
diff --git a/tests/README.md b/tests/README.md
new file mode 100644
index 0000000..f8c1899
--- /dev/null
+++ b/tests/README.md
@@ -0,0 +1,18 @@
+
+# smatch tests
+
+This directory contains test cases to verify the correct behavior of
+smatch. Run the tests with [pytest](https://pytest.org/). This will
+require you to install both pytest and smatch:
+
+```console
+$ pip install pytest
+$ pip install -e .  # current directory is smatch
+$ pytest
+```
+
+**Note:** As smatch is inherently non-deterministic due to its
+hill-climbing implementation, the tests can be "flaky" (i.e.,
+sometimes pass, sometimes fail). To mitigate the possibility of flaky
+tests, test cases should use *minimal* AMRs so it becomes trivial for
+smatch to get the optimal solution.
diff --git a/tests/test_top.py b/tests/test_top.py
new file mode 100644
index 0000000..a9bf4c4
--- /dev/null
+++ b/tests/test_top.py
@@ -0,0 +1,57 @@
+
+"""
+The top node in an AMR determines the focus of the encoded
+meaning. For example, the following have the same dependencies but a
+different node at the top:
+
+    (w / white-03
+       :ARG1 (m / marble))
+
+The above AMR means "the marble is white" or "the whiteness of the
+marble".
+
+    (m / marble
+       :ARG1-of (w / white-03))
+
+This AMR means "the marble that is white" or "the white marble".
+
+For this reason, AMRs that differ only in which node is the top will
+get smatch scores less than 1.0.
+
+For more information see:
+
+    https://github.com/amrisi/amr-guidelines/blob/master/amr.md#focus
+"""
+
+import smatch
+
+a = '(a / alpha :ARG0 (b / beta))'
+b = '(a / alternative :ARG0 (b / beta))'
+c = '(a / alpha :ARG0 (b / b-side))'
+d = '(b / beta :ARG0-of (a / alpha))'
+
+
+def get_amr_match(amr1, amr2):
+    vals = smatch.get_amr_match(amr1, amr2)
+    smatch.match_triple_dict.clear()
+    return vals
+
+
+def test_same():
+    assert get_amr_match(a, a) == (4, 4, 4)
+    smatch.match_triple_dict.clear()
+
+
+def test_same_top_different_top_concept():
+    assert get_amr_match(a, b) == (3, 4, 4)
+    smatch.match_triple_dict.clear()
+
+
+def test_same_top_different_dependent_concept():
+    assert get_amr_match(a, c) == (3, 4, 4)
+    smatch.match_triple_dict.clear()
+
+
+def test_same_different_top():
+    assert get_amr_match(a, d) == (3, 4, 4)
+    smatch.match_triple_dict.clear()