Integrate UnicodeDammit, deprecate parser_class, deprecate encodeValue, refactor, scaffolding for more unit tests

docs/user_guide/advanced.rst

@@ -211,8 +211,6 @@ Here is a full list of the configuration options:
 
 ``MAX_FILE_MEMO``, default 20000, "python setup.py sdist bdist_wininst upload"
 
-``parser_class``, default 'lxml', "lxml vs soup"
-
 ``memoize_articles``, default True, "cache and save articles run after run"
 
 ``fetch_images``, default True, "set this to false if you don't care about getting images"

newspaper/article.py

@@ -19,7 +19,7 @@
 from .configuration import Configuration
 from .extractors import ContentExtractor
 from .outputformatters import OutputFormatter
-from .utils import (URLHelper, encodeValue, RawHelper, extend_config,
+from .utils import (URLHelper, RawHelper, extend_config,
                     get_available_languages)
 from .videos.extractors import VideoExtractor
 
@@ -49,12 +49,12 @@ def __init__(self, url, title='', source_url='', config=None, **kwargs):
             raise ArticleException('input url bad format')
 
         # URL to the main page of the news source which owns this article
-        self.source_url = encodeValue(source_url)
+        self.source_url = source_url
 
-        url = encodeValue(url)
+        url = url
         self.url = urls.prepare_url(url, self.source_url)
 
-        self.title = encodeValue(title)
+        self.title = title
 
         # URL of the "best image" to represent this article
         self.top_img = self.top_image = ''
@@ -375,13 +375,12 @@ def set_title(self, title):
             # <title> extraction failed
             return
         title = title[:self.config.MAX_TITLE]
-        title = encodeValue(title)
+        title = title
         if title:
             self.title = title
 
     def set_text(self, text):
         text = text[:self.config.MAX_TEXT]
-        text = encodeValue(text)
         if text:
             self.text = text
 
@@ -390,16 +389,16 @@ def set_html(self, html):
         """
         self.is_downloaded = True
         if html:
-            self.html = encodeValue(html)
+            self.html = html
 
     def set_article_html(self, article_html):
         """Sets the HTML of just the article's `top_node`
         """
         if article_html:
-            self.article_html = encodeValue(article_html)
+            self.article_html = article_html
 
     def set_meta_img(self, src_url):
-        self.meta_img = encodeValue(src_url)
+        self.meta_img = src_url
         self.set_top_img_no_check(src_url)
 
     def set_top_img(self, src_url):
@@ -412,15 +411,13 @@ def set_top_img_no_check(self, src_url):
         """Provide 2 APIs for images. One at "top_img", "imgs"
         and one at "top_image", "images"
         """
-        src_url = encodeValue(src_url)
         self.top_img = src_url
         self.top_image = src_url
 
     def set_imgs(self, imgs):
         """The motive for this method is the same as above, provide APIs
         for both `article.imgs` and `article.images`
         """
-        imgs = [encodeValue(i) for i in imgs]
         self.images = imgs
         self.imgs = imgs
 
@@ -430,24 +427,21 @@ def set_keywords(self, keywords):
         if not isinstance(keywords, list):
             raise Exception("Keyword input must be list!")
         if keywords:
-            self.keywords = [encodeValue(k)
-                             for k in keywords[:self.config.MAX_KEYWORDS]]
+            self.keywords = keywords[:self.config.MAX_KEYWORDS]
 
     def set_authors(self, authors):
         """Authors are in ["firstName lastName", "firstName lastName"] format
         """
         if not isinstance(authors, list):
             raise Exception("authors input must be list!")
         if authors:
-            authors = authors[:self.config.MAX_AUTHORS]
-            self.authors = [encodeValue(author) for author in authors]
+            self.authors = authors[:self.config.MAX_AUTHORS]
 
     def set_summary(self, summary):
         """Summary here refers to a paragraph of text from the
         title text and body text
         """
-        summary = summary[:self.config.MAX_SUMMARY]
-        self.summary = encodeValue(summary)
+        self.summary = summary[:self.config.MAX_SUMMARY]
 
     def set_meta_language(self, meta_lang):
         """Save langauges in their ISO 2-character form

newspaper/configuration.py

@@ -12,7 +12,7 @@
 
 import logging
 
-from .parsers import Parser, ParserSoup
+from .parsers import Parser
 from .text import (StopWords, StopWordsArabic, StopWordsChinese,
                    StopWordsKorean)
 from .version import __version__
@@ -38,8 +38,6 @@ def __init__(self):
         # max number of urls we cache for each news source
         self.MAX_FILE_MEMO = 20000
 
-        self.parser_class = 'lxml'  # 'lxml' or 'soup'
-
         # Cache and save articles run after run
         self.memoize_articles = True
 
@@ -104,7 +102,7 @@ def get_stopwords_class(self, language):
         return StopWords
 
     def get_parser(self):
-        return Parser if self.parser_class == 'lxml' else ParserSoup
+        return Parser
 
 
 class ArticleConfiguration(Configuration):

newspaper/extractors.py

@@ -352,7 +352,13 @@ def get_meta_data(self, doc):
                 continue
 
             key = key.split(':')
-            ref = data[key.pop(0)]
+            key_head = key.pop(0)
+            ref = data[key_head]
+
+            if isinstance(ref, str):
+                data[key_head] = {key_head: ref}
+                ref = data[key_head]
+
             for idx, part in enumerate(key):
                 if idx == len(key) - 1:
                     ref[part] = value
@@ -390,7 +396,8 @@ def get_img_urls(self, article_url, doc):
         img_tags = self.parser.getElementsByTag(doc, **img_kwargs)
         urls = [img_tag.get('src')
                 for img_tag in img_tags if img_tag.get('src')]
-        img_links = set([urllib.parse.urljoin(article_url, url) for url in urls])
+        img_links = set([urllib.parse.urljoin(article_url, url)
+                        for url in urls])
         return img_links
 
     def get_first_img_url(self, article_url, top_node):
@@ -436,7 +443,7 @@ def get_urls(self, doc_or_html, titles=False, regex=False):
             doc_or_html = [i.strip() for i in doc_or_html]
             return doc_or_html or []
         # If the doc_or_html is html, parse it into a root
-        if isinstance(doc_or_html, str) or isinstance(doc_or_html, str):
+        if isinstance(doc_or_html, str):
             doc = self.parser.fromstring(doc_or_html)
         else:
             doc = doc_or_html
@@ -457,7 +464,8 @@ def get_category_urls(self, source_url, doc):
 
             if not domain and not path:
                 if self.config.verbose:
-                    print('elim category url %s for no domain and path' % p_url)
+                    print('elim category url %s for no domain and path'
+                          % p_url)
                 continue
             if path and path.startswith('#'):
                 if self.config.verbose:

newspaper/images.py

@@ -12,8 +12,9 @@
 import math
 import io
 import traceback
-import urllib.request, urllib.parse, urllib.error
-import urllib.request, urllib.error, urllib.parse
+import urllib.request
+import urllib.parse
+import urllib.error
 
 from http.client import InvalidURL
 from PIL import Image, ImageFile
@@ -79,7 +80,8 @@ def clean_url(url):
     """Url quotes unicode data out of urls
     """
     url = url.encode('utf8')
-    url = ''.join([urllib.parse.quote(c) if ord(c) >= 127 else c for c in url.decode('utf-8')])
+    url = ''.join([urllib.parse.quote(c)
+                  if ord(c) >= 127 else c for c in url.decode('utf-8')])
     return url
 
 
@@ -151,7 +153,9 @@ def fetch_url(url, useragent, referer=None, retries=1, dimension=False):
 
             return content_type, content
 
-        except (urllib.error.URLError, urllib.error.HTTPError, InvalidURL) as e:
+        except (urllib.error.URLError,
+                urllib.error.HTTPError,
+                InvalidURL) as e:
             cur_try += 1
             if cur_try >= retries:
                 log.debug('error while fetching: %s refer: %s' %

newspaper/mthreading.py

@@ -101,7 +101,8 @@ def join(self):
         resets the task.
         """
         if self.pool is None:
-            print('Call set(..) with a list of source objects before .join(..)')
+            print('Call set(..) with a list of source '
+                  'objects before .join(..)')
             raise
         self.pool.wait_completion()
         self.papers = []

newspaper/parsers.py

@@ -13,10 +13,10 @@
 import re
 import traceback
 
+from bs4 import UnicodeDammit
 from copy import deepcopy
 
 from . import text
-from . import utils
 
 log = logging.getLogger(__name__)
 
@@ -41,29 +41,30 @@ def drop_tag(cls, nodes):
     def css_select(cls, node, selector):
         return node.cssselect(selector)
 
+    @classmethod
+    def get_unicode_html(cls, html):
+        converted = UnicodeDammit(html, is_html=True)
+        if not converted.unicode_markup:
+            raise Exception(
+                'Failed to detect encoding of article HTML, tried: %s' %
+                ', '.join(converted.tried_encodings))
+        html = converted.unicode_markup
+        return html
+
     @classmethod
     def fromstring(cls, html):
-        html = utils.encodeValue(html)
-        # don't bring the entire library down because one article
-        # or article failed to parse
+        html = cls.get_unicode_html(html)
+        # Enclosed in a `try` to prevent bringing the entire library
+        # down due to one article (out of potentially many in a `Source`)
         try:
-            # remove encoding tag because lxml won't accept it for
-            # unicode objects (Issue #78)
-            if isinstance(html, bytes):
-                if html.startswith(b'<?'):
-                    html = re.sub(b'^\<\?.*?\?\>', b'', html, flags=re.DOTALL)
-            else:
-                if html.startswith('<?'):
-                    html = re.sub(r'^\<\?.*?\?\>', '', html, flags=re.DOTALL)
+            # lxml does not play well with <? ?> encoding tags
+            if html.startswith('<?'):
+                html = re.sub(r'^\<\?.*?\?\>', '', html, flags=re.DOTALL)
             cls.doc = lxml.html.fromstring(html)
             return cls.doc
         except Exception:
             traceback.print_exc()
-            return None
-
-    # @classmethod
-    # def set_doc(cls, html):
-    #    cls.doc = cls.fromstring(html)
+            return
 
     @classmethod
     def node_to_string(cls, node):
@@ -83,6 +84,9 @@ def clean_article_html(cls, node):
 
     @classmethod
     def nodeToString(cls, node):
+        """`decode` is needed at the end because `etree.tostring`
+        returns a python bytestring
+        """
         return lxml.etree.tostring(node).decode()
 
     @classmethod
@@ -261,11 +265,3 @@ def outerHtml(cls, node):
             e0 = deepcopy(e0)
             e0.tail = None
         return cls.nodeToString(e0)
-
-
-class ParserSoup(Parser):
-    @classmethod
-    def fromstring(cls, html):
-        html = utils.encodeValue(html)
-        cls.doc = lxml.html.soupparser.fromstring(html)
-        return cls.doc

newspaper/source.py

@@ -30,15 +30,15 @@
 class Category(object):
 
     def __init__(self, url):
-        self.url = utils.encodeValue(url)
+        self.url = url
         self.html = None
         self.doc = None
 
 
 class Feed(object):
 
     def __init__(self, url):
-        self.url = utils.encodeValue(url)
+        self.url = url
         self.rss = None
         # TODO self.dom = None, speed up Feedparser
 
@@ -64,7 +64,7 @@ def __init__(self, url, config=None, **kwargs):
 
         self.extractor = ContentExtractor(self.config)
 
-        self.url = utils.encodeValue(url)
+        self.url = url
         self.url = urls.prepare_url(url)
 
         self.domain = urls.get_domain(self.url)
@@ -141,7 +141,7 @@ def set_description(self):
         desc html attribute
         """
         desc = self.extractor.get_meta_description(self.doc)
-        self.description = utils.encodeValue(desc)
+        self.description = desc
 
     def download(self, response=None):
         """Downloads html of source
@@ -338,8 +338,8 @@ def download_articles(self, threads=1):
         self.is_downloaded = True
         if len(failed_articles) > 0:
             if self.config.verbose:
-                print('[ERROR], these article urls failed the download:', \
-                    [a.url for a in failed_articles])
+                print('[ERROR], these article urls failed the download:',
+                      [a.url for a in failed_articles])
 
     def parse_articles(self):
         """Parse all articles, delete if too small