Repository: andycasey/ads

Issue:
Exception handling in Unicode representation of Articles
In the article method `__unicode__()`, the article properties `first_author`, `bibcode` and `year` are used. This can yield an exception if the fields are not included in the original search query; generally for `first_author` as no getter exists, or if deferred loading for `bibcode` and `year` fails.

Cf. pull request #55 for a more detailed discussion of the issue.

Discussion:

Patch:
diff --git a/ads/search.py b/ads/search.py
index c8a0bb4..8f36421 100644
--- a/ads/search.py
+++ b/ads/search.py
@@ -40,21 +40,20 @@ class Article(object):
         return self.__unicode__().encode("utf-8")
 
     def __unicode__(self):
-        author = self.first_author or "Unknown author"
-        if self.author and len(self.author) > 1:
+        author = self._raw.get("first_author", "Unknown author")
+        if len(self._raw.get("author", [])) > 1:
             author += " et al."
 
         return u"<{author} {year}, {bibcode}>".format(
             author=author,
-            year=self.year,
-            bibcode=self.bibcode,
+            year=self._raw.get("year", "Unknown year"),
+            bibcode=self._raw.get("bibcode", "Unknown bibcode")
         )
 
     def __eq__(self, other):
-        if (not hasattr(self, 'bibcode') or not hasattr(other, 'bibcode') or
-                self.bibcode is None or other.bibcode is None):
+        if self._raw.get("bibcode") is None or other._raw.get("bibcode") is None:
             raise TypeError("Cannot compare articles without bibcodes")
-        return self.bibcode == other.bibcode
+        return self._raw['bibcode'] == other._raw['bibcode']
 
     def __ne__(self, other):
         return not self.__eq__(other)
@@ -196,8 +195,8 @@ class Article(object):
         return self._get_field('indexstamp')
 
     @cached_property
-    def first_author_norm(self):
-        return self._get_field('first_author_norm')
+    def first_author(self):
+        return self._get_field('first_author')
 
     @cached_property
     def issue(self):


Test patch:
diff --git a/ads/tests/test_search.py b/ads/tests/test_search.py
index dc36eda..75d834f 100644
--- a/ads/tests/test_search.py
+++ b/ads/tests/test_search.py
@@ -49,12 +49,13 @@ class TestArticle(unittest.TestCase):
     def test_equals(self):
         """
         the __eq__ method should compare bibcodes, and raise if bibcode isn't
-        defined
+        defined or is None
         """
         self.assertNotEqual(Article(bibcode="Not the same"), self.article)
         self.assertEqual(Article(bibcode="2013A&A...552A.143S"), self.article)
-        with self.assertRaises(TypeError):
-            # Explicitly set bibcode to None to avoid invoking the getter.
+        with self.assertRaisesRegexp(TypeError, "Cannot compare articles without bibcodes"):
+            Article() == self.article
+        with self.assertRaisesRegexp(TypeError, "Cannot compare articles without bibcodes"):
             Article(bibcode=None) == self.article
 
     def test_init(self):
@@ -79,6 +80,10 @@ class TestArticle(unittest.TestCase):
             self.article.__str__()
         )
         self.assertEqual(self.article.__unicode__(), self.article.__str__())
+        self.assertEqual(
+            Article().__str__(),
+            "<Unknown author Unknown year, Unknown bibcode>"
+        )
 
     @patch('ads.search.Article._get_field')
     def test_cached_properties(self, patched):