Repository: DarkEnergySurvey/mkauthlist

Issue:
Python 3 byte strings
The Python 3 `str` object is based on Unicode, which means that `file.write(..., 'wb')` doesn't work anymore. We need to change this, but we should probably also understand why we were specifying `'wb'` in the first place.

Some documentation on the Python 3 change:
https://www.python.org/dev/peps/pep-0404/#strings-and-bytes
https://stackoverflow.com/a/33054552/4075339

And the specific place that needs to be changed: [L137](https://github.com/DarkEnergySurvey/mkauthlist/blob/1fb62affeaa73c5192cce84323f77f66d466306a/mkauthlist/mkauthlist.py#L137)


Discussion:

Patch:
diff --git a/.gitignore b/.gitignore
index 5b6ffac..086cdcc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -10,4 +10,5 @@ dist
 *.out
 *.aux
 *.log
-*.spl
\ No newline at end of file
+*.spl
+*.cls
\ No newline at end of file
diff --git a/data/author_order.csv b/data/author_order.csv
index 3749293..de4b8f3 100644
--- a/data/author_order.csv
+++ b/data/author_order.csv
@@ -1,5 +1,6 @@
 Melchior
 Sheldon, Erin
+#Commented, Name
 Drlica-Wagner
 Rykoff
 Plazas Malagón
\ No newline at end of file
diff --git a/data/example_author_list.csv b/data/example_author_list.csv
index 4efafbb..ccbc4ed 100644
--- a/data/example_author_list.csv
+++ b/data/example_author_list.csv
@@ -4,7 +4,7 @@ Drlica-Wagner,Alex,A.~Drlica-Wagner,False,"Fermi National Accelerator Laboratory
 Rykoff,Eli,E.~S.~Rykoff,False,"Kavli Institute for Particle Astrophysics \& Cosmology, P. O. Box 2450, Stanford University, Stanford, CA 94305, USA","Data quality expert",
 Rykoff,Eli,E.~S.~Rykoff,False,"SLAC National Accelerator Laboratory, Menlo Park, CA 94025, USA","Data quality expert",
 Sheldon,Erin,E.~Sheldon,False,"Brookhaven National Laboratory, Bldg 510, Upton, NY 11973, USA","Data backend",
-Abbott,Tim,T. M. C.~Abbott,True,"Cerro Tololo Inter-American Observatory, National Optical Astronomy Observatory, Casilla 603, La Serena, Chile",,
+Zhang,Yuanyuan,Y.~Zhang,True,"Fermi National Accelerator Laboratory, P. O. Box 500, Batavia, IL 60510, USA",,
 Abdalla,Filipe,F.~B.~Abdalla,True,"Department of Physics \& Astronomy, University College London, Gower Street, London, WC1E 6BT, UK",,
 Abdalla,Filipe,F.~B.~Abdalla,True,"Department of Physics and Electronics, Rhodes University, PO Box 94, Grahamstown, 6140, South Africa",,
 Allam,Sahar,S.~Allam,True,"Fermi National Accelerator Laboratory, P. O. Box 500, Batavia, IL 60510, USA",,
@@ -66,4 +66,4 @@ Tarle,Gregory,G.~Tarle,True,"Department of Physics, University of Michigan, Ann
 Vikram,Vinu,V.~Vikram,True,"Argonne National Laboratory, 9700 South Cass Avenue, Lemont, IL 60439, USA",,
 Walker,Alistair,A.~R.~Walker,True,"Cerro Tololo Inter-American Observatory, National Optical Astronomy Observatory, Casilla 603, La Serena, Chile",,
 Wester,William,W.~Wester,True,"Fermi National Accelerator Laboratory, P. O. Box 500, Batavia, IL 60510, USA",,
-Zhang,Yuanyuan,Y.~Zhang,True,"Fermi National Accelerator Laboratory, P. O. Box 500, Batavia, IL 60510, USA",,
\ No newline at end of file
+Abbott,Tim,T.~M.~C.~Abbott,True,"Cerro Tololo Inter-American Observatory, National Optical Astronomy Observatory, Casilla 603, La Serena, Chile",,
diff --git a/mkauthlist/mkauthlist.py b/mkauthlist/mkauthlist.py
index 1393e93..4745d3a 100755
--- a/mkauthlist/mkauthlist.py
+++ b/mkauthlist/mkauthlist.py
@@ -134,7 +134,7 @@ def write_contributions(filename,data):
 
     logging.info('Writing contribution file: %s'%filename)
 
-    out = open(filename,'wb')
+    out = open(filename,'w')
     out.write(output)
     out.close()
 
@@ -374,7 +374,7 @@ if __name__ == "__main__":
     affidict = odict()
     authdict = odict()
 
-    # Hack for Munich affiliation...
+    # Hack for umlauts in affiliations...
     for k,v in HACK.items():
         logging.warn("Hacking '%s' ..."%k)
         select = (np.char.count(data['Affiliation'],k) > 0)
@@ -382,13 +382,18 @@ if __name__ == "__main__":
 
     # Pre-sort the csv file by the auxiliary file
     if args.aux is not None:
-        aux = [r for r in csv.DictReader(open(args.aux),['Lastname','Firstname'])]
+        auxcols = ['Lastname','Firstname']
+        aux = [[r[c] for c in auxcols] for r in 
+               csv.DictReader(open(args.aux),fieldnames=auxcols) 
+               if not r[auxcols[0]].startswith('#')]
+        aux = np.rec.fromrecords(aux,names=auxcols)
         if len(np.unique(aux)) != len(aux):
             logging.error('Non-unique names in aux file.')
             print(open(args.aux).read())
             raise Exception()
-            
-        raw = np.array(zip(data['Lastname'],range(len(data))))
+
+        # Ugh, python2/3 compatibility
+        raw = np.array(list(zip(data['Lastname'],list(range(len(data))))))
         order = np.empty((0,2),dtype=raw.dtype)
         for r in aux:
             lastname = r['Lastname']


Test patch:
diff --git a/tests/test_authlist.py b/tests/test_authlist.py
index 958472e..cb57ba3 100644
--- a/tests/test_authlist.py
+++ b/tests/test_authlist.py
@@ -9,7 +9,7 @@ import logging
 import subprocess
 import unittest
 
-class TestAuthlistFunc(unittest.TestCase):
+class TestAuthlist(unittest.TestCase):
 
     def setUp(self):
         self.csv = 'example_author_list.csv'
@@ -32,12 +32,13 @@ class TestAuthlistFunc(unittest.TestCase):
         #    shutil.copy(os.path.join('data',filename),'.')
 
     def tearDown(self):
-        self.clean = [self.csv,self.tex,self.aux,self.out,self.log,self.bib,self.pdf,self.order]
+        self.clean = [self.csv,self.tex,self.aux,self.out,self.log,self.bib,
+                      self.pdf,self.order,self.cntrb]
         self.clean += self.cls
 
         cmd = "rm -f "+' '.join(self.clean)
         print(cmd)
-        subprocess.check_output(cmd,shell=True)
+        #subprocess.check_output(cmd,shell=True)
 
     def latex(self, tex=None, pdf=None):
         if tex is None: tex = self.tex
@@ -49,35 +50,76 @@ class TestAuthlistFunc(unittest.TestCase):
         shutil.copy(tex.replace('.tex','.pdf'),pdf)
         
     def test_mkauthlist(self):
+        """Run 'vanilla' mkauthlist."""
         cmd = "mkauthlist -f --doc %(csv)s %(tex)s"%self.files
         print(cmd)
         subprocess.check_output(cmd,shell=True)
         self.latex(pdf='test_mkauthlist.pdf')
 
-    def test_author_order(self):
+    def test_order(self):
+        """Explicitly order some authors."""
         cmd = "mkauthlist -f --doc %(csv)s %(tex)s -a %(order)s"%self.files
         print(cmd)
         subprocess.check_output(cmd,shell=True)
-        self.latex(pdf='test_order.pdf')
+
+        # Shouldn't be any need to build the file
+        #self.latex(pdf='test_order.pdf')
+
+        with open(self.tex,'r') as f:
+            authors = [l for l in f.readlines() if l.startswith('\\author')]
+            self.assertEqual(authors[1],'\\author{E.~Sheldon}\n')
+            self.assertEqual(authors[4],'\\author{A.~A.~Plazas}\n')
+            self.assertEqual(authors[5],'\\author{Y.~Zhang}\n')
+            self.assertEqual(authors[-1],'\\author{T.~M.~C.~Abbott}\n')
 
     def test_contribution(self):
+        """Write author contributions."""
         cmd = "mkauthlist -f --doc %(csv)s %(tex)s --cntrb %(cntrb)s"%self.files
         print(cmd)
         subprocess.check_output(cmd,shell=True)
-        self.latex(pdf='test_contrib.pdf')
 
-        if not os.path.exists(self.cntrb): 
-            msg = "No contributions found"
-            raise Exception(msg)
-        
-        with open(self.cntrb) as cntrb:
-            lines = cntrb.readlines()
-            msg = "Unexpected author contributions: "
-            if not lines[0].split()[0] == 'Author':
-                raise Exception(msg+'\n'+lines[0])
-                msg = "Unexpected author contributions"
-            if not lines[1].split()[0] == 'P.~Melchior:':
-                raise Exception(msg+'\n'+lines[1])
+        # Shouldn't be any need to build the file
+        #self.latex(pdf='test_contrib.pdf')
+
+        with open(self.cntrb,'r') as f:
+            lines = f.readlines()
+            self.assertEqual(lines[0],'Author contributions are listed below. \\\\\n')
+            self.assertEqual(lines[1],'P.~Melchior: Lead designer and author \\\\\n')
+            self.assertEqual(lines[-1],'T.~M.~C.~Abbott:  \\\\\n')
+
+    def test_sort(self):
+        """Sort all authors alphabetically."""
+        cmd = "mkauthlist -f --doc %(csv)s %(tex)s --sort"%self.files
+        print(cmd)
+        subprocess.check_output(cmd,shell=True)
+
+        with open(self.tex,'r') as f:
+            authors = [l for l in f.readlines() if l.startswith('\\author')]
+            self.assertEqual(authors[0],'\\author{T.~M.~C.~Abbott}\n')
+            self.assertEqual(authors[-1],'\\author{Y.~Zhang}\n')
+
+    def test_sort_order(self):
+        """Order some authors, sort the rest."""
+        cmd = "mkauthlist -f --doc %(csv)s %(tex)s --sort -a %(order)s"%self.files
+        print(cmd)
+        subprocess.check_output(cmd,shell=True)
+
+        with open(self.tex,'r') as f:
+            authors = [l for l in f.readlines() if l.startswith('\\author')]
+            self.assertEqual(authors[1],'\\author{E.~Sheldon}\n')
+            self.assertEqual(authors[-1],'\\author{Y.~Zhang}\n')
+
+    def test_sort_builder(self):
+        """Sort builders, but leave other authors unchanged."""
+        cmd = "mkauthlist -f --doc %(csv)s %(tex)s -sb"%self.files
+        print(cmd)
+        subprocess.check_output(cmd,shell=True)
+
+        with open(self.tex,'r') as f:
+            authors = [l for l in f.readlines() if l.startswith('\\author')]
+            self.assertEqual(authors[3],'\\author{E.~Sheldon}\n')
+            self.assertEqual(authors[4],'\\author{T.~M.~C.~Abbott}\n')
+            self.assertEqual(authors[-1],'\\author{Y.~Zhang}\n')
             
 if __name__ == "__main__":
     unittest.main()
diff --git a/tests/test_journals.py b/tests/test_journals.py
index 09c7f86..4833263 100755
--- a/tests/test_journals.py
+++ b/tests/test_journals.py
@@ -9,7 +9,7 @@ import logging
 import subprocess
 import unittest
 
-class TestJournalFunc(unittest.TestCase):
+class TestJournal(unittest.TestCase):
 
     def setUp(self):
         self.csv = 'example_author_list.csv'