From 4cef1a1d0060148716ffec4ad6ce9bf709ad1ea2 Mon Sep 17 00:00:00 2001
From: Pawel Miech <pawelmhm@gmail.com>
Date: Fri, 13 May 2016 12:35:35 +0200
Subject: [PATCH 01/14] [image_pipeline] bring back uppercase pipeline
 attributes

allow users to have class attributes on image pipelines. This assumes
that class attributes are useful if users want to have different pipeline
classes inhriting from ImagePipeline.
---
 scrapy/pipelines/files.py           |  9 ++++++---
 scrapy/pipelines/images.py          | 21 +++++++++++++--------
 scrapy/settings/default_settings.py |  7 -------
 tests/test_pipeline_images.py       | 12 ++++++++----
 4 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py
index c94794173..3e6ad554d 100644
--- a/scrapy/pipelines/files.py
+++ b/scrapy/pipelines/files.py
@@ -214,11 +214,14 @@ class FilesPipeline(MediaPipeline):
     """
 
     MEDIA_NAME = "file"
+    EXPIRES = 90
     STORE_SCHEMES = {
         '': FSFilesStore,
         'file': FSFilesStore,
         's3': S3FilesStore,
     }
+    DEFAULT_FILES_URLS_FIELD = 'file_urls'
+    DEFAULT_FILES_RESULT_FIELD = 'files'
 
     def __init__(self, store_uri, download_func=None, settings=None):
         if not store_uri:
@@ -228,9 +231,9 @@ class FilesPipeline(MediaPipeline):
             settings = Settings(settings)
         
         self.store = self._get_store(store_uri)
-        self.expires = settings.getint('FILES_EXPIRES')
-        self.files_urls_field = settings.get('FILES_URLS_FIELD')
-        self.files_result_field = settings.get('FILES_RESULT_FIELD')
+        self.expires = settings.getint('FILES_EXPIRES', self.EXPIRES)
+        self.files_urls_field = settings.get('FILES_URLS_FIELD', self.DEFAULT_FILES_URLS_FIELD)
+        self.files_result_field = settings.get('FILES_RESULT_FIELD', self.DEFAULT_FILES_RESULT_FIELD)
 
         super(FilesPipeline, self).__init__(download_func=download_func)
 
diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py
index c597b6cca..ac78ee6c0 100644
--- a/scrapy/pipelines/images.py
+++ b/scrapy/pipelines/images.py
@@ -37,26 +37,31 @@ class ImagesPipeline(FilesPipeline):
     """
 
     MEDIA_NAME = 'image'
+    MIN_WIDTH = 0
+    MIN_HEIGHT = 0
+    EXPIRES = 0
+    THUMBS = {}
+    IMAGES_URLS_FIELD = 'image_urls'
+    IMAGES_RESULT_FIELD = 'images'
 
     def __init__(self, store_uri, download_func=None, settings=None):
         super(ImagesPipeline, self).__init__(store_uri, settings=settings, download_func=download_func)
-        
+
         if isinstance(settings, dict) or settings is None:
             settings = Settings(settings)
 
-        self.expires = settings.getint('IMAGES_EXPIRES')
-        self.images_urls_field = settings.get('IMAGES_URLS_FIELD')
-        self.images_result_field = settings.get('IMAGES_RESULT_FIELD')
-        self.min_width = settings.getint('IMAGES_MIN_WIDTH')
-        self.min_height = settings.getint('IMAGES_MIN_HEIGHT')
-        self.thumbs = settings.get('IMAGES_THUMBS')
+        self.expires = settings.getint('IMAGES_EXPIRES', self.EXPIRES)
+        self.images_urls_field = settings.get('IMAGES_URLS_FIELD', self.IMAGES_URLS_FIELD)
+        self.images_result_field = settings.get('IMAGES_RESULT_FIELD', self.IMAGES_RESULT_FIELD)
+        self.min_width = settings.getint('IMAGES_MIN_WIDTH', self.MIN_WIDTH)
+        self.min_height = settings.getint('IMAGES_MIN_HEIGHT', self.MIN_HEIGHT)
+        self.thumbs = settings.get('IMAGES_THUMBS', self.THUMBS)
 
     @classmethod
     def from_settings(cls, settings):
         s3store = cls.STORE_SCHEMES['s3']
         s3store.AWS_ACCESS_KEY_ID = settings['AWS_ACCESS_KEY_ID']
         s3store.AWS_SECRET_ACCESS_KEY = settings['AWS_SECRET_ACCESS_KEY']
-
         store_uri = settings['IMAGES_STORE']
         return cls(store_uri, settings=settings)
 
diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py
index 8f064f81e..1e447e9e2 100644
--- a/scrapy/settings/default_settings.py
+++ b/scrapy/settings/default_settings.py
@@ -179,13 +179,6 @@ HTTPCACHE_GZIP = False
 
 HTTPPROXY_AUTH_ENCODING = 'latin-1'
 
-IMAGES_MIN_WIDTH = 0
-IMAGES_MIN_HEIGHT = 0
-IMAGES_EXPIRES = 90
-IMAGES_THUMBS = {}
-IMAGES_URLS_FIELD = 'image_urls'
-IMAGES_RESULT_FIELD = 'images'
-
 ITEM_PROCESSOR = 'scrapy.pipelines.ItemPipelineManager'
 
 ITEM_PIPELINES = {}
diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py
index f48547b0f..3ce138000 100644
--- a/tests/test_pipeline_images.py
+++ b/tests/test_pipeline_images.py
@@ -224,19 +224,22 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase):
     def test_images_urls_field(self):
         another_pipeline = ImagesPipeline.from_settings(Settings({'IMAGES_STORE': self.tempdir,
                                                                 'IMAGES_URLS_FIELD': 'funny_field'}))
-        self.assertEqual(self.pipeline.images_urls_field, self.default_settings.get('IMAGES_URLS_FIELD'))
+        default = self.pipeline.IMAGES_URLS_FIELD
+        self.assertEqual(self.pipeline.images_urls_field, self.default_settings.get('IMAGES_URLS_FIELD', default))
         self.assertEqual(another_pipeline.images_urls_field, 'funny_field')
 
     def test_images_result_field(self):
         another_pipeline = ImagesPipeline.from_settings(Settings({'IMAGES_STORE': self.tempdir,
                                                                 'IMAGES_RESULT_FIELD': 'funny_field'}))
-        self.assertEqual(self.pipeline.images_result_field, self.default_settings.get('IMAGES_RESULT_FIELD'))
+        default = self.pipeline.IMAGES_RESULT_FIELD
+        self.assertEqual(self.pipeline.images_result_field, self.default_settings.get('IMAGES_RESULT_FIELD', default))
         self.assertEqual(another_pipeline.images_result_field, 'funny_field')
 
     def test_min_width(self):
         another_pipeline = ImagesPipeline.from_settings(Settings({'IMAGES_STORE': self.tempdir,
                                                                 'IMAGES_MIN_WIDTH': 42}))
-        self.assertEqual(self.pipeline.min_width, self.default_settings.getint('IMAGES_MIN_WIDTH'))
+        default = self.pipeline.MIN_WIDTH
+        self.assertEqual(self.pipeline.min_width, self.default_settings.getint('IMAGES_MIN_WIDTH', default))
         self.assertEqual(another_pipeline.min_width, 42)
 
     def test_min_height(self):
@@ -249,7 +252,8 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase):
         custom_thumbs = {'small': (50, 50), 'big': (270, 270)}
         another_pipeline = ImagesPipeline.from_settings(Settings({'IMAGES_STORE': self.tempdir,
                                                                 'IMAGES_THUMBS': custom_thumbs}))
-        self.assertEqual(self.pipeline.thumbs, self.default_settings.get('IMAGES_THUMBS'))
+        default = self.pipeline.THUMBS
+        self.assertEqual(self.pipeline.thumbs, self.default_settings.get('IMAGES_THUMBS', default))
         self.assertEqual(another_pipeline.thumbs, custom_thumbs)
 
 

From 6c67db3917dd07761b113f3e3b37ebf5a9558f00 Mon Sep 17 00:00:00 2001
From: Pawel Miech <pawelmhm@gmail.com>
Date: Wed, 18 May 2016 12:04:52 +0200
Subject: [PATCH 02/14] [image_pipeline] tests for class attrs backward
 compatibility

and docs about image pipeline settings.
---
 docs/topics/media-pipeline.rst |  4 ++++
 tests/test_pipeline_images.py  | 23 +++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst
index 3da243d29..2b41a4f43 100644
--- a/docs/topics/media-pipeline.rst
+++ b/docs/topics/media-pipeline.rst
@@ -191,6 +191,10 @@ For the Images Pipeline, set :setting:`IMAGES_URLS_FIELD` and/or
 If you need something more complex and want to override the custom pipeline
 behaviour, see :ref:`topics-media-pipeline-override`.
 
+.. note:: If you have multiple image pipelines and you want to have different settings in different pipelines
+   you can either set class attributes in pipeline object or add custom settings keys and load them at pipeline
+   initialization.
+
 
 Additional features
 ===================
diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py
index 3ce138000..49257e175 100644
--- a/tests/test_pipeline_images.py
+++ b/tests/test_pipeline_images.py
@@ -256,6 +256,29 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase):
         self.assertEqual(self.pipeline.thumbs, self.default_settings.get('IMAGES_THUMBS', default))
         self.assertEqual(another_pipeline.thumbs, custom_thumbs)
 
+    def test_class_attrs_preserved(self):
+
+        class UserDefinedImagePipeline(ImagesPipeline):
+            MIN_WIDTH = 1000
+
+        # If image settings are not defined values are taken from class attributes.
+        pipeline = UserDefinedImagePipeline.from_settings(Settings({"IMAGES_STORE": self.tempdir}))
+        self.assertEqual(pipeline.min_width, 1000)
+
+    def test_class_attrs_not_preserved_if_settings_defined(self):
+
+        class UserDefinedImagePipeline(ImagesPipeline):
+            MIN_WIDTH = 1000
+
+        settings = {
+            "IMAGES_STORE": self.tempdir,
+            "IMAGES_MIN_WIDTH": 90
+        }
+
+        # If image settings are defined they override class attributes.
+        pipeline = UserDefinedImagePipeline.from_settings(Settings(settings))
+        self.assertEqual(pipeline.min_width, 90)
+
 
 def _create_image(format, *a, **kw):
     buf = TemporaryFile()

From a62d4b081c8eef1e54c6f7399e7a4154e3f330f3 Mon Sep 17 00:00:00 2001
From: Pawel Miech <pawelmhm@gmail.com>
Date: Fri, 10 Jun 2016 12:48:02 +0200
Subject: [PATCH 03/14] [image-pipeline] image settings with class name

allow to have image settings with class name, so that settings for user defined ImagePipeline
subclasses can be defined easily.
---
 docs/topics/media-pipeline.rst |  6 +++---
 scrapy/pipelines/images.py     | 27 +++++++++++++++++++++------
 tests/test_pipeline_images.py  | 18 ++++++++++++++++--
 3 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst
index 2b41a4f43..96339d03d 100644
--- a/docs/topics/media-pipeline.rst
+++ b/docs/topics/media-pipeline.rst
@@ -191,9 +191,9 @@ For the Images Pipeline, set :setting:`IMAGES_URLS_FIELD` and/or
 If you need something more complex and want to override the custom pipeline
 behaviour, see :ref:`topics-media-pipeline-override`.
 
-.. note:: If you have multiple image pipelines and you want to have different settings in different pipelines
-   you can either set class attributes in pipeline object or add custom settings keys and load them at pipeline
-   initialization.
+.. note:: If you have multiple image pipelines inheriting from ImagePipeline and you want to have different settings in different pipelines
+   you can set setting keys preceded with uppercase name of your pipeline class. E.g. if your pipeline is called
+   MyPipeline and you want to have custom IMAGES_URLS_FIELD you define setting MYPIPELINE_IMAGES_URLS_FIELD.
 
 
 Additional features
diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py
index ac78ee6c0..9ba04750a 100644
--- a/scrapy/pipelines/images.py
+++ b/scrapy/pipelines/images.py
@@ -50,12 +50,27 @@ class ImagesPipeline(FilesPipeline):
         if isinstance(settings, dict) or settings is None:
             settings = Settings(settings)
 
-        self.expires = settings.getint('IMAGES_EXPIRES', self.EXPIRES)
-        self.images_urls_field = settings.get('IMAGES_URLS_FIELD', self.IMAGES_URLS_FIELD)
-        self.images_result_field = settings.get('IMAGES_RESULT_FIELD', self.IMAGES_RESULT_FIELD)
-        self.min_width = settings.getint('IMAGES_MIN_WIDTH', self.MIN_WIDTH)
-        self.min_height = settings.getint('IMAGES_MIN_HEIGHT', self.MIN_HEIGHT)
-        self.thumbs = settings.get('IMAGES_THUMBS', self.THUMBS)
+        def key_for_pipe(key):
+            """
+            Allow setting settings for user defined ImagePipelines that inherit from base.
+
+            User can define setting key:
+
+            MYPIPELINENAME_IMAGE_SETTING_NAME = <some value>
+
+            and it will override default settings and class attributes.
+            """
+            class_name = self.__class__.__name__
+            if class_name == "ImagesPipeline":
+                return key
+            return "{}_{}".format(class_name.upper(), key)
+
+        self.expires = settings.getint(key_for_pipe('IMAGES_EXPIRES'), self.EXPIRES)
+        self.images_urls_field = settings.get(key_for_pipe('IMAGES_URLS_FIELD'), self.IMAGES_URLS_FIELD)
+        self.images_result_field = settings.get(key_for_pipe('IMAGES_RESULT_FIELD'), self.IMAGES_RESULT_FIELD)
+        self.min_width = settings.getint(key_for_pipe('IMAGES_MIN_WIDTH'), self.MIN_WIDTH)
+        self.min_height = settings.getint(key_for_pipe('IMAGES_MIN_HEIGHT'), self.MIN_HEIGHT)
+        self.thumbs = settings.get(key_for_pipe('IMAGES_THUMBS'), self.THUMBS)
 
     @classmethod
     def from_settings(cls, settings):
diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py
index 49257e175..3b68faed0 100644
--- a/tests/test_pipeline_images.py
+++ b/tests/test_pipeline_images.py
@@ -265,7 +265,7 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase):
         pipeline = UserDefinedImagePipeline.from_settings(Settings({"IMAGES_STORE": self.tempdir}))
         self.assertEqual(pipeline.min_width, 1000)
 
-    def test_class_attrs_not_preserved_if_settings_defined(self):
+    def test_class_attrs_preserved_if_only_global_settings_defined(self):
 
         class UserDefinedImagePipeline(ImagesPipeline):
             MIN_WIDTH = 1000
@@ -277,7 +277,21 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase):
 
         # If image settings are defined they override class attributes.
         pipeline = UserDefinedImagePipeline.from_settings(Settings(settings))
-        self.assertEqual(pipeline.min_width, 90)
+        self.assertEqual(pipeline.min_width, 1000)
+
+    def test_settings_multiple_pipelilines(self):
+        # If user has multiple pipelines he can define setting keys preceded with
+        # pipeline class name.
+        class UserDefinedPipeline(ImagesPipeline):
+            pass
+
+        settings = {
+            "IMAGES_MIN_WIDTH": 10,
+            "USERDEFINEDPIPELINE_IMAGES_MIN_WIDTH": 1999,
+            "IMAGES_STORE": self.tempdir
+        }
+        user_pipeline = UserDefinedPipeline.from_settings(Settings(settings))
+        self.assertEqual(user_pipeline.min_width, 1999)
 
 
 def _create_image(format, *a, **kw):

From d715172528925ec0242d29122a0cad6a6f4fd4e2 Mon Sep 17 00:00:00 2001
From: Pawel Miech <pawelmhm@gmail.com>
Date: Tue, 14 Jun 2016 19:09:56 +0200
Subject: [PATCH 04/14] [image_pipeline] unify and simplify tests for setting
 loading

there was identical test for different setting keys. I unified it into
one unit test.

Fixes comments for tests, adds comments about intention of uppercase attrs.

Adds another test for user defined setting keys and uppercase attrs.
---
 scrapy/pipelines/images.py    |  3 ++
 tests/test_pipeline_images.py | 77 ++++++++++++++++-------------------
 2 files changed, 39 insertions(+), 41 deletions(-)

diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py
index 9ba04750a..b9a594e23 100644
--- a/scrapy/pipelines/images.py
+++ b/scrapy/pipelines/images.py
@@ -37,6 +37,9 @@ class ImagesPipeline(FilesPipeline):
     """
 
     MEDIA_NAME = 'image'
+
+    # Uppercase attributes kept for backward compatibility with code that subclasses
+    # ImagesPipeline. They may be overriden by settings.
     MIN_WIDTH = 0
     MIN_HEIGHT = 0
     EXPIRES = 0
diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py
index 3b68faed0..69e2eb82f 100644
--- a/tests/test_pipeline_images.py
+++ b/tests/test_pipeline_images.py
@@ -207,6 +207,7 @@ class ImagesPipelineTestCaseFields(unittest.TestCase):
 
 class ImagesPipelineTestCaseCustomSettings(unittest.TestCase):
 
+
     def setUp(self):
         self.tempdir = mkdtemp()
         self.pipeline = ImagesPipeline(self.tempdir)
@@ -215,46 +216,27 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase):
     def tearDown(self):
         rmtree(self.tempdir)
 
-    def test_expires(self):
-        another_pipeline = ImagesPipeline.from_settings(Settings({'IMAGES_STORE': self.tempdir,
-                                                                'IMAGES_EXPIRES': 42}))
-        self.assertEqual(self.pipeline.expires, self.default_settings.getint('IMAGES_EXPIRES'))
-        self.assertEqual(another_pipeline.expires, 42)
-
-    def test_images_urls_field(self):
-        another_pipeline = ImagesPipeline.from_settings(Settings({'IMAGES_STORE': self.tempdir,
-                                                                'IMAGES_URLS_FIELD': 'funny_field'}))
-        default = self.pipeline.IMAGES_URLS_FIELD
-        self.assertEqual(self.pipeline.images_urls_field, self.default_settings.get('IMAGES_URLS_FIELD', default))
-        self.assertEqual(another_pipeline.images_urls_field, 'funny_field')
-
-    def test_images_result_field(self):
-        another_pipeline = ImagesPipeline.from_settings(Settings({'IMAGES_STORE': self.tempdir,
-                                                                'IMAGES_RESULT_FIELD': 'funny_field'}))
-        default = self.pipeline.IMAGES_RESULT_FIELD
-        self.assertEqual(self.pipeline.images_result_field, self.default_settings.get('IMAGES_RESULT_FIELD', default))
-        self.assertEqual(another_pipeline.images_result_field, 'funny_field')
-
-    def test_min_width(self):
-        another_pipeline = ImagesPipeline.from_settings(Settings({'IMAGES_STORE': self.tempdir,
-                                                                'IMAGES_MIN_WIDTH': 42}))
-        default = self.pipeline.MIN_WIDTH
-        self.assertEqual(self.pipeline.min_width, self.default_settings.getint('IMAGES_MIN_WIDTH', default))
-        self.assertEqual(another_pipeline.min_width, 42)
-
-    def test_min_height(self):
-        another_pipeline = ImagesPipeline.from_settings(Settings({'IMAGES_STORE': self.tempdir,
-                                                                'IMAGES_MIN_HEIGHT': 42}))
-        self.assertEqual(self.pipeline.min_height, self.default_settings.getint('IMAGES_MIN_HEIGHT'))
-        self.assertEqual(another_pipeline.min_height, 42)
-
-    def test_thumbs(self):
-        custom_thumbs = {'small': (50, 50), 'big': (270, 270)}
-        another_pipeline = ImagesPipeline.from_settings(Settings({'IMAGES_STORE': self.tempdir,
-                                                                'IMAGES_THUMBS': custom_thumbs}))
-        default = self.pipeline.THUMBS
-        self.assertEqual(self.pipeline.thumbs, self.default_settings.get('IMAGES_THUMBS', default))
-        self.assertEqual(another_pipeline.thumbs, custom_thumbs)
+    def test_different_settings_for_different_instances(self):
+        custom_settings = [
+            # Order is: key name in settings.py, value, name of pipeline attribute.
+            ("IMAGES_EXPIRES", 42, "EXPIRES"),
+            ("IMAGES_STORE", self.tempdir, "IMAGES_STORE"),
+            ("IMAGES_RESULT_FIELD", "funny_field", "IMAGES_RESULT_FIELD"),
+            ("IMAGES_URLS_FIELD", "other_field", "IMAGES_URLS_FIELD"),
+            ("IMAGES_MIN_WIDTH", 99, "MIN_WIDTH"),
+            ("IMAGES_MIN_HEIGHT", 112, "MIN_HEIGHT"),
+            ("IMAGES_THUMBS", {'small': (50, 50), 'big': (270, 270)}, "THUMBS")
+        ]
+        default_settings = Settings()
+        default_sts_pipe = ImagesPipeline(self.tempdir, settings=default_settings)
+        user_sts_pipe = ImagesPipeline.from_settings(Settings({k: v for k, v, _ in custom_settings}))
+        for key, custom_value, attr_name in custom_settings:
+            if attr_name == "IMAGES_STORE":
+                # this is not set as pipeline attribute
+                continue
+            expected_default_value = getattr(default_sts_pipe, attr_name)
+            self.assertEqual(getattr(default_sts_pipe, attr_name), expected_default_value, key)
+            self.assertEqual(getattr(user_sts_pipe, attr_name.lower()), custom_value, key)
 
     def test_class_attrs_preserved(self):
 
@@ -275,7 +257,7 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase):
             "IMAGES_MIN_WIDTH": 90
         }
 
-        # If image settings are defined they override class attributes.
+        # Class attributes for subclass of ImagePipeline override default setting keys.
         pipeline = UserDefinedImagePipeline.from_settings(Settings(settings))
         self.assertEqual(pipeline.min_width, 1000)
 
@@ -293,6 +275,19 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase):
         user_pipeline = UserDefinedPipeline.from_settings(Settings(settings))
         self.assertEqual(user_pipeline.min_width, 1999)
 
+    def test_settings_multiple_pipelilines_and_class_attrs(self):
+        # Setting keys for user defined pipeline override class attributes.
+        class UserDefinedPipeline(ImagesPipeline):
+            MIN_WIDTH = 200
+
+        settings = {
+            "IMAGES_MIN_WIDTH": 10,
+            "USERDEFINEDPIPELINE_IMAGES_MIN_WIDTH": 1999,
+            "IMAGES_STORE": self.tempdir
+        }
+        user_pipeline = UserDefinedPipeline.from_settings(Settings(settings))
+        self.assertEqual(user_pipeline.min_width, 1999)
+
 
 def _create_image(format, *a, **kw):
     buf = TemporaryFile()

From ee39d11e45a6ca20a24d28cd08c4b20ea1eae40e Mon Sep 17 00:00:00 2001
From: Pawel Miech <pawelmhm@gmail.com>
Date: Wed, 15 Jun 2016 11:25:38 +0200
Subject: [PATCH 05/14] [image_pipeline] refactor and simplify tests for image
 settings

unify tests that test same thing for different attribute values into one. Add
better docstrings for tests.
---
 scrapy/pipelines/images.py    |   2 +-
 tests/test_pipeline_images.py | 188 ++++++++++++++++++++++------------
 2 files changed, 125 insertions(+), 65 deletions(-)

diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py
index b9a594e23..465d7c492 100644
--- a/scrapy/pipelines/images.py
+++ b/scrapy/pipelines/images.py
@@ -39,7 +39,7 @@ class ImagesPipeline(FilesPipeline):
     MEDIA_NAME = 'image'
 
     # Uppercase attributes kept for backward compatibility with code that subclasses
-    # ImagesPipeline. They may be overriden by settings.
+    # ImagesPipeline. They may be overridden by settings.
     MIN_WIDTH = 0
     MIN_HEIGHT = 0
     EXPIRES = 0
diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py
index 69e2eb82f..177a887d0 100644
--- a/tests/test_pipeline_images.py
+++ b/tests/test_pipeline_images.py
@@ -1,5 +1,6 @@
 import os
 import hashlib
+import random
 import warnings
 from tempfile import mkdtemp, TemporaryFile
 from shutil import rmtree
@@ -206,87 +207,146 @@ class ImagesPipelineTestCaseFields(unittest.TestCase):
 
 
 class ImagesPipelineTestCaseCustomSettings(unittest.TestCase):
+    img_cls_attribute_names = [
+        # Pipeline attribute names with corresponding setting names.
+        ("EXPIRES", "IMAGES_EXPIRES"),
+        ("MIN_WIDTH", "IMAGES_MIN_WIDTH"),
+        ("MIN_HEIGHT", "IMAGES_MIN_HEIGHT"),
+        ("IMAGES_URLS_FIELD", "IMAGES_URLS_FIELD"),
+        ("IMAGES_RESULT_FIELD", "IMAGES_RESULT_FIELD"),
+        ("THUMBS", "IMAGES_THUMBS")
+    ]
+
+    # This should match what is defined in ImagesPipeline.
+    default_pipeline_settings = dict(
+        MIN_WIDTH=0,
+        MIN_HEIGHT=0,
+        EXPIRES=0,
+        THUMBS={},
+        IMAGES_URLS_FIELD='image_urls',
+        IMAGES_RESULT_FIELD='images'
+    )
 
 
     def setUp(self):
         self.tempdir = mkdtemp()
-        self.pipeline = ImagesPipeline(self.tempdir)
-        self.default_settings = Settings()
 
     def tearDown(self):
         rmtree(self.tempdir)
 
+    def _generate_fake_settings(self, prefix=None):
+        """
+        :param prefix: string for setting keys
+        :return: dictionary of image pipeline settings
+        """
+
+        def random_string():
+            return "".join([chr(random.randint(97, 123)) for _ in range(10)])
+
+        settings = {
+            "IMAGES_EXPIRES": random.randint(1, 1000),
+            "IMAGES_STORE": self.tempdir,
+            "IMAGES_RESULT_FIELD": random_string(),
+            "IMAGES_URLS_FIELD": random_string(),
+            "IMAGES_MIN_WIDTH": random.randint(1, 1000),
+            "IMAGES_MIN_HEIGHT": random.randint(1, 1000),
+            "IMAGES_THUMBS": {
+                'small': (random.randint(1, 1000), random.randint(1, 1000)),
+                'big': (random.randint(1, 1000), random.randint(1, 1000))
+            }
+        }
+        if not prefix:
+            return settings
+
+        return {prefix.upper() + "_" + k if k != "IMAGES_STORE" else k: v for k, v in settings.items()}
+
+    def _generate_fake_pipeline_subclass(self):
+        """
+        :return: ImagePipeline class will all uppercase attributes set.
+        """
+        class UserDefinedImagePipeline(ImagesPipeline):
+            # Values should be in different range than fake_settings.
+            MIN_WIDTH = random.randint(1000, 2000)
+            MIN_HEIGHT = random.randint(1000, 2000)
+            THUMBS = {
+                'small': (random.randint(1000, 2000), random.randint(1000, 2000)),
+                'big': (random.randint(1000, 2000), random.randint(1000, 2000))
+            }
+            EXPIRES = random.randint(1000, 2000)
+            IMAGES_URLS_FIELD = "field_one"
+            IMAGES_RESULT_FIELD = "field_two"
+
+        return UserDefinedImagePipeline
+
     def test_different_settings_for_different_instances(self):
-        custom_settings = [
-            # Order is: key name in settings.py, value, name of pipeline attribute.
-            ("IMAGES_EXPIRES", 42, "EXPIRES"),
-            ("IMAGES_STORE", self.tempdir, "IMAGES_STORE"),
-            ("IMAGES_RESULT_FIELD", "funny_field", "IMAGES_RESULT_FIELD"),
-            ("IMAGES_URLS_FIELD", "other_field", "IMAGES_URLS_FIELD"),
-            ("IMAGES_MIN_WIDTH", 99, "MIN_WIDTH"),
-            ("IMAGES_MIN_HEIGHT", 112, "MIN_HEIGHT"),
-            ("IMAGES_THUMBS", {'small': (50, 50), 'big': (270, 270)}, "THUMBS")
-        ]
+        """
+        If there are two instances of ImagesPipeline class with different settings, they should
+        have different settings.
+        """
+        custom_settings = self._generate_fake_settings()
         default_settings = Settings()
         default_sts_pipe = ImagesPipeline(self.tempdir, settings=default_settings)
-        user_sts_pipe = ImagesPipeline.from_settings(Settings({k: v for k, v, _ in custom_settings}))
-        for key, custom_value, attr_name in custom_settings:
-            if attr_name == "IMAGES_STORE":
-                # this is not set as pipeline attribute
-                continue
-            expected_default_value = getattr(default_sts_pipe, attr_name)
-            self.assertEqual(getattr(default_sts_pipe, attr_name), expected_default_value, key)
-            self.assertEqual(getattr(user_sts_pipe, attr_name.lower()), custom_value, key)
+        user_sts_pipe = ImagesPipeline.from_settings(Settings(custom_settings))
+        for pipe_attr, settings_attr in self.img_cls_attribute_names:
+            expected_default_value = self.default_pipeline_settings.get(pipe_attr)
+            custom_value = custom_settings.get(settings_attr)
+            self.assertNotEqual(expected_default_value, custom_value)
+            self.assertEqual(getattr(default_sts_pipe, pipe_attr.lower()), expected_default_value)
+            self.assertEqual(getattr(user_sts_pipe, pipe_attr.lower()), custom_value)
 
-    def test_class_attrs_preserved(self):
+    def test_subclass_attrs_preserved_default_settings(self):
+        """
+        If image settings are not defined at all subclass of ImagePipeline takes values
+        from class attributes.
+        """
+        pipeline_cls = self._generate_fake_pipeline_subclass()
+        pipeline = pipeline_cls.from_settings(Settings({"IMAGES_STORE": self.tempdir}))
+        for pipe_attr, settings_attr in self.img_cls_attribute_names:
+            # Instance attribute (lowercase) must be equal to class attribute (uppercase).
+            attr_value = getattr(pipeline, pipe_attr.lower())
+            self.assertEqual(attr_value, getattr(pipeline, pipe_attr))
 
+    def test_subclass_attrs_preserved_custom_settings(self):
+        """
+        If image settings are defined but they are not defined for subclass class attributes
+        should be preserved.
+        """
+        pipeline_cls = self._generate_fake_pipeline_subclass()
+        settings = self._generate_fake_settings()
+        pipeline = pipeline_cls.from_settings(Settings(settings))
+        for pipe_attr, settings_attr in self.img_cls_attribute_names:
+            # Instance attribute (lowercase) must be equal to class attribute (uppercase).
+            value = getattr(pipeline, pipe_attr.lower())
+            self.assertEqual(value, getattr(pipeline, pipe_attr))
+
+    def test_custom_settings_for_subclasses(self):
+        """
+        If there are custom settings for subclass and NO class attributes, pipeline should use custom
+        settings.
+        """
         class UserDefinedImagePipeline(ImagesPipeline):
-            MIN_WIDTH = 1000
-
-        # If image settings are not defined values are taken from class attributes.
-        pipeline = UserDefinedImagePipeline.from_settings(Settings({"IMAGES_STORE": self.tempdir}))
-        self.assertEqual(pipeline.min_width, 1000)
-
-    def test_class_attrs_preserved_if_only_global_settings_defined(self):
-
-        class UserDefinedImagePipeline(ImagesPipeline):
-            MIN_WIDTH = 1000
-
-        settings = {
-            "IMAGES_STORE": self.tempdir,
-            "IMAGES_MIN_WIDTH": 90
-        }
-
-        # Class attributes for subclass of ImagePipeline override default setting keys.
-        pipeline = UserDefinedImagePipeline.from_settings(Settings(settings))
-        self.assertEqual(pipeline.min_width, 1000)
-
-    def test_settings_multiple_pipelilines(self):
-        # If user has multiple pipelines he can define setting keys preceded with
-        # pipeline class name.
-        class UserDefinedPipeline(ImagesPipeline):
             pass
 
-        settings = {
-            "IMAGES_MIN_WIDTH": 10,
-            "USERDEFINEDPIPELINE_IMAGES_MIN_WIDTH": 1999,
-            "IMAGES_STORE": self.tempdir
-        }
-        user_pipeline = UserDefinedPipeline.from_settings(Settings(settings))
-        self.assertEqual(user_pipeline.min_width, 1999)
+        prefix = UserDefinedImagePipeline.__name__.upper()
+        settings = self._generate_fake_settings(prefix=prefix)
+        user_pipeline = UserDefinedImagePipeline.from_settings(Settings(settings))
+        for pipe_attr, settings_attr in self.img_cls_attribute_names:
+            # Values from settings for custom pipeline should be set on pipeline instance.
+            custom_value = settings.get(prefix + "_" + settings_attr)
+            self.assertEqual(getattr(user_pipeline, pipe_attr.lower()), custom_value)
 
-    def test_settings_multiple_pipelilines_and_class_attrs(self):
-        # Setting keys for user defined pipeline override class attributes.
-        class UserDefinedPipeline(ImagesPipeline):
-            MIN_WIDTH = 200
-
-        settings = {
-            "IMAGES_MIN_WIDTH": 10,
-            "USERDEFINEDPIPELINE_IMAGES_MIN_WIDTH": 1999,
-            "IMAGES_STORE": self.tempdir
-        }
-        user_pipeline = UserDefinedPipeline.from_settings(Settings(settings))
-        self.assertEqual(user_pipeline.min_width, 1999)
+    def test_custom_settings_and_class_attrs_for_subclasses(self):
+        """
+        If there are custom settings for subclass AND class attributes
+        setting keys are preferred and override attributes.
+        """
+        pipeline_cls = self._generate_fake_pipeline_subclass()
+        prefix = pipeline_cls.__name__.upper()
+        settings = self._generate_fake_settings(prefix=prefix)
+        user_pipeline = pipeline_cls.from_settings(Settings(settings))
+        for pipe_attr, settings_attr in self.img_cls_attribute_names:
+            custom_value = settings.get(prefix + "_" + settings_attr)
+            self.assertEqual(getattr(user_pipeline, pipe_attr.lower()), custom_value)
 
 
 def _create_image(format, *a, **kw):

From 72e4d5f33ef8130acf833031b018192c39b60152 Mon Sep 17 00:00:00 2001
From: Pawel Miech <pawelmhm@gmail.com>
Date: Wed, 15 Jun 2016 14:07:17 +0200
Subject: [PATCH 06/14] [image_pipeline] another test for subclass inheritance

test case when subclass inherits from base class and has no attributes nor
settings defined.
---
 tests/test_pipeline_images.py | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py
index 177a887d0..a2dd5aa28 100644
--- a/tests/test_pipeline_images.py
+++ b/tests/test_pipeline_images.py
@@ -319,6 +319,20 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase):
             value = getattr(pipeline, pipe_attr.lower())
             self.assertEqual(value, getattr(pipeline, pipe_attr))
 
+    def test_no_custom_settings_for_subclasses(self):
+        """
+        If there are no settings for subclass and no subclass attributes, pipeline should use
+        attributes of base class.
+        """
+        class UserDefinedImagePipeline(ImagesPipeline):
+            pass
+
+        user_pipeline = UserDefinedImagePipeline.from_settings(Settings({"IMAGES_STORE": self.tempdir}))
+        for pipe_attr, settings_attr in self.img_cls_attribute_names:
+            # Values from settings for custom pipeline should be set on pipeline instance.
+            custom_value = self.default_pipeline_settings.get(pipe_attr.upper())
+            self.assertEqual(getattr(user_pipeline, pipe_attr.lower()), custom_value)
+
     def test_custom_settings_for_subclasses(self):
         """
         If there are custom settings for subclass and NO class attributes, pipeline should use custom

From c6d1686d98f640be87df4337015e83fba2964f53 Mon Sep 17 00:00:00 2001
From: Pawel Miech <pawelmhm@gmail.com>
Date: Wed, 15 Jun 2016 14:48:25 +0200
Subject: [PATCH 07/14] [files_pipeline] unify tests for files pipeline

if test tests same thing but for different field it can be unified into one.
---
 tests/test_pipeline_files.py | 52 ++++++++++++++++++++++++------------
 1 file changed, 35 insertions(+), 17 deletions(-)

diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py
index 391538562..760684c2b 100644
--- a/tests/test_pipeline_files.py
+++ b/tests/test_pipeline_files.py
@@ -1,4 +1,5 @@
 import os
+import random
 import time
 import hashlib
 import warnings
@@ -184,32 +185,49 @@ class FilesPipelineTestCaseFields(unittest.TestCase):
 
 
 class FilesPipelineTestCaseCustomSettings(unittest.TestCase):
+    default_cls_settings = {
+        "EXPIRES": 90,
+        "DEFAULT_FILES_URLS_FIELD": "file_urls",
+        "DEFAULT_FILES_RESULT_FIELD": "files"
+    }
+    file_cls_attr_settings_map = {
+        ("EXPIRES", "FILES_EXPIRES"),
+        ("DEFAULT_FILES_URLS_FIELD", "FILES_URLS_FIELD"),
+        ("DEFAULT_FILES_RESULT_FIELD", "FILES_RESULT_FIELD")
+    }
 
     def setUp(self):
         self.tempdir = mkdtemp()
-        self.pipeline = FilesPipeline(self.tempdir)
-        self.default_settings = Settings()
 
     def tearDown(self):
         rmtree(self.tempdir)
 
-    def test_expires(self):
-        another_pipeline = FilesPipeline.from_settings(Settings({'FILES_STORE': self.tempdir,
-                                                                'FILES_EXPIRES': 42}))
-        self.assertEqual(self.pipeline.expires, self.default_settings.getint('FILES_EXPIRES'))
-        self.assertEqual(another_pipeline.expires, 42)
+    def _generate_fake_settings(self, prefix=None):
 
-    def test_files_urls_field(self):
-        another_pipeline = FilesPipeline.from_settings(Settings({'FILES_STORE': self.tempdir,
-                                                                'FILES_URLS_FIELD': 'funny_field'}))
-        self.assertEqual(self.pipeline.files_urls_field, self.default_settings.get('FILES_URLS_FIELD'))
-        self.assertEqual(another_pipeline.files_urls_field, 'funny_field')
+        def random_string():
+            return "".join([chr(random.randint(97, 123)) for _ in range(10)])
 
-    def test_files_result_field(self):
-        another_pipeline = FilesPipeline.from_settings(Settings({'FILES_STORE': self.tempdir,
-                                                                'FILES_RESULT_FIELD': 'funny_field'}))
-        self.assertEqual(self.pipeline.files_result_field, self.default_settings.get('FILES_RESULT_FIELD'))
-        self.assertEqual(another_pipeline.files_result_field, 'funny_field')
+        settings = {
+            "FILES_EXPIRES": random.randint(1, 1000),
+            "FILES_URLS_FIELD": random_string(),
+            "FILES_RESULT_FIELD": random_string(),
+            "FILES_STORE": self.tempdir
+        }
+        if not prefix:
+            return settings
+
+        return {prefix.upper() + "_" + k: v for k, v in settings.items()}
+
+    def test_different_settings_for_different_instances(self):
+        custom_settings = self._generate_fake_settings()
+        another_pipeline = FilesPipeline.from_settings(Settings(custom_settings))
+        one_pipeline = FilesPipeline(self.tempdir)
+        for pipe_attr, settings_attr in self.file_cls_attr_settings_map:
+            default_value = self.default_cls_settings[pipe_attr]
+            self.assertEqual(getattr(one_pipeline, pipe_attr), default_value)
+            custom_value = custom_settings[settings_attr]
+            pipe_attr_lower = pipe_attr.lower().replace("default_", "")
+            self.assertEqual(getattr(another_pipeline, pipe_attr_lower), custom_value)
 
 
 class TestS3FilesStore(unittest.TestCase):

From acbfdc618496f304c1a81f5f07ff0cacabfe8394 Mon Sep 17 00:00:00 2001
From: Pawel Miech <pawelmhm@gmail.com>
Date: Wed, 15 Jun 2016 15:12:18 +0200
Subject: [PATCH 08/14] [files_pipeline] ensure class attributes are preserved

dont override class attributes with default settings (same as in image pipeline).
---
 scrapy/settings/default_settings.py |  3 ---
 tests/test_pipeline_files.py        | 23 +++++++++++++++++++++++
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py
index 1e447e9e2..b9d01e155 100644
--- a/scrapy/settings/default_settings.py
+++ b/scrapy/settings/default_settings.py
@@ -160,9 +160,6 @@ FEED_EXPORTERS_BASE = {
 }
 
 FILES_STORE_S3_ACL = 'private'
-FILES_EXPIRES = 90
-FILES_URLS_FIELD = 'file_urls'
-FILES_RESULT_FIELD = 'files'
 
 HTTPCACHE_ENABLED = False
 HTTPCACHE_DIR = 'httpcache'
diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py
index 760684c2b..4c64f6f3e 100644
--- a/tests/test_pipeline_files.py
+++ b/tests/test_pipeline_files.py
@@ -218,7 +218,20 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase):
 
         return {prefix.upper() + "_" + k: v for k, v in settings.items()}
 
+    def _generate_fake_pipeline(self):
+
+        class UserDefinedFilePipeline(FilesPipeline):
+            FILES_EXPIRES = random.randint(1001, 2000)
+            DEFAULT_FILES_URLS_FIELD = "alfa"
+            DEFAULT_FILES_RESULT_FIELD = "beta"
+
+        return UserDefinedFilePipeline
+
     def test_different_settings_for_different_instances(self):
+        """
+        If there are different instances with different settings they should keep
+        different settings.
+        """
         custom_settings = self._generate_fake_settings()
         another_pipeline = FilesPipeline.from_settings(Settings(custom_settings))
         one_pipeline = FilesPipeline(self.tempdir)
@@ -229,6 +242,16 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase):
             pipe_attr_lower = pipe_attr.lower().replace("default_", "")
             self.assertEqual(getattr(another_pipeline, pipe_attr_lower), custom_value)
 
+    def test_subclass_attributes_preserved_if_no_settings(self):
+        """
+        If subclasses override class attributes and there are no special settings those values should be kept.
+        """
+        pipe_cls = self._generate_fake_pipeline()
+        pipe = pipe_cls.from_settings(Settings({"FILES_STORE": self.tempdir}))
+        for pipe_attr, settings_attr in self.file_cls_attr_settings_map:
+            attr_lower = pipe_attr.lower().replace("default_", "")
+            self.assertEqual(getattr(pipe, attr_lower), getattr(pipe, pipe_attr))
+
 
 class TestS3FilesStore(unittest.TestCase):
     @defer.inlineCallbacks

From 539d34bce08c6c0bf19f8ae31e9d77271c3e22b7 Mon Sep 17 00:00:00 2001
From: Pawel Miech <pawelmhm@gmail.com>
Date: Wed, 15 Jun 2016 15:39:11 +0200
Subject: [PATCH 09/14] [media-pipeline, file-pipeline] allow setting custom
 settings for subclasses

* move key_for_pipe function to media pipeline so that file pipeline can use it
* use key_for_pipe in file pipeline so that users can define custom settings for subclasses easily
* add tests for file pipelines attributes and settings
---
 scrapy/pipelines/files.py    | 15 ++++++--
 scrapy/pipelines/images.py   | 40 ++++++++++----------
 scrapy/pipelines/media.py    | 16 ++++++++
 tests/test_pipeline_files.py | 73 +++++++++++++++++++++++++++++++-----
 4 files changed, 109 insertions(+), 35 deletions(-)

diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py
index 3e6ad554d..b9c43dc3b 100644
--- a/scrapy/pipelines/files.py
+++ b/scrapy/pipelines/files.py
@@ -229,11 +229,18 @@ class FilesPipeline(MediaPipeline):
         
         if isinstance(settings, dict) or settings is None:
             settings = Settings(settings)
-        
+
+        cls_name = "FilesPipeline"
         self.store = self._get_store(store_uri)
-        self.expires = settings.getint('FILES_EXPIRES', self.EXPIRES)
-        self.files_urls_field = settings.get('FILES_URLS_FIELD', self.DEFAULT_FILES_URLS_FIELD)
-        self.files_result_field = settings.get('FILES_RESULT_FIELD', self.DEFAULT_FILES_RESULT_FIELD)
+        self.expires = settings.getint(
+            self._key_for_pipe('FILES_EXPIRES', cls_name), self.EXPIRES
+        )
+        self.files_urls_field = settings.get(
+            self._key_for_pipe('FILES_URLS_FIELD', cls_name), self.DEFAULT_FILES_URLS_FIELD
+        )
+        self.files_result_field = settings.get(
+            self._key_for_pipe('FILES_RESULT_FIELD', cls_name), self.DEFAULT_FILES_RESULT_FIELD
+        )
 
         super(FilesPipeline, self).__init__(download_func=download_func)
 
diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py
index 465d7c492..de616211e 100644
--- a/scrapy/pipelines/images.py
+++ b/scrapy/pipelines/images.py
@@ -53,27 +53,25 @@ class ImagesPipeline(FilesPipeline):
         if isinstance(settings, dict) or settings is None:
             settings = Settings(settings)
 
-        def key_for_pipe(key):
-            """
-            Allow setting settings for user defined ImagePipelines that inherit from base.
-
-            User can define setting key:
-
-            MYPIPELINENAME_IMAGE_SETTING_NAME = <some value>
-
-            and it will override default settings and class attributes.
-            """
-            class_name = self.__class__.__name__
-            if class_name == "ImagesPipeline":
-                return key
-            return "{}_{}".format(class_name.upper(), key)
-
-        self.expires = settings.getint(key_for_pipe('IMAGES_EXPIRES'), self.EXPIRES)
-        self.images_urls_field = settings.get(key_for_pipe('IMAGES_URLS_FIELD'), self.IMAGES_URLS_FIELD)
-        self.images_result_field = settings.get(key_for_pipe('IMAGES_RESULT_FIELD'), self.IMAGES_RESULT_FIELD)
-        self.min_width = settings.getint(key_for_pipe('IMAGES_MIN_WIDTH'), self.MIN_WIDTH)
-        self.min_height = settings.getint(key_for_pipe('IMAGES_MIN_HEIGHT'), self.MIN_HEIGHT)
-        self.thumbs = settings.get(key_for_pipe('IMAGES_THUMBS'), self.THUMBS)
+        cls_name = "ImagesPipeline"
+        self.expires = settings.getint(
+            self._key_for_pipe('IMAGES_EXPIRES', cls_name), self.EXPIRES
+        )
+        self.images_urls_field = settings.get(
+            self._key_for_pipe('IMAGES_URLS_FIELD', cls_name), self.IMAGES_URLS_FIELD
+        )
+        self.images_result_field = settings.get(
+            self._key_for_pipe('IMAGES_RESULT_FIELD', cls_name), self.IMAGES_RESULT_FIELD
+        )
+        self.min_width = settings.getint(
+            self._key_for_pipe('IMAGES_MIN_WIDTH', cls_name), self.MIN_WIDTH
+        )
+        self.min_height = settings.getint(
+            self._key_for_pipe('IMAGES_MIN_HEIGHT', cls_name), self.MIN_HEIGHT
+        )
+        self.thumbs = settings.get(
+            self._key_for_pipe('IMAGES_THUMBS', cls_name), self.THUMBS
+        )
 
     @classmethod
     def from_settings(cls, settings):
diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py
index 21b8b8986..740312f8f 100644
--- a/scrapy/pipelines/media.py
+++ b/scrapy/pipelines/media.py
@@ -27,6 +27,22 @@ class MediaPipeline(object):
     def __init__(self, download_func=None):
         self.download_func = download_func
 
+
+    def _key_for_pipe(self, key, base_class_name):
+        """
+        Allow setting settings for user defined MediaPipelines that inherit from base.
+
+        User can define setting key:
+
+        MYPIPELINENAME_IMAGE_SETTING_NAME = <some value>
+
+        and it will override default settings and class attributes.
+        """
+        class_name = self.__class__.__name__
+        if class_name == base_class_name:
+            return key
+        return "{}_{}".format(class_name.upper(), key)
+
     @classmethod
     def from_crawler(cls, crawler):
         try:
diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py
index 4c64f6f3e..fd54b7229 100644
--- a/tests/test_pipeline_files.py
+++ b/tests/test_pipeline_files.py
@@ -191,9 +191,9 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase):
         "DEFAULT_FILES_RESULT_FIELD": "files"
     }
     file_cls_attr_settings_map = {
-        ("EXPIRES", "FILES_EXPIRES"),
-        ("DEFAULT_FILES_URLS_FIELD", "FILES_URLS_FIELD"),
-        ("DEFAULT_FILES_RESULT_FIELD", "FILES_RESULT_FIELD")
+        ("EXPIRES", "FILES_EXPIRES", "expires"),
+        ("DEFAULT_FILES_URLS_FIELD", "FILES_URLS_FIELD", "files_urls_field"),
+        ("DEFAULT_FILES_RESULT_FIELD", "FILES_RESULT_FIELD", "files_result_field")
     }
 
     def setUp(self):
@@ -216,7 +216,7 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase):
         if not prefix:
             return settings
 
-        return {prefix.upper() + "_" + k: v for k, v in settings.items()}
+        return {prefix.upper() + "_" + k if k != "FILES_STORE" else k: v for k, v in settings.items()}
 
     def _generate_fake_pipeline(self):
 
@@ -235,12 +235,11 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase):
         custom_settings = self._generate_fake_settings()
         another_pipeline = FilesPipeline.from_settings(Settings(custom_settings))
         one_pipeline = FilesPipeline(self.tempdir)
-        for pipe_attr, settings_attr in self.file_cls_attr_settings_map:
+        for pipe_attr, settings_attr, pipe_ins_attr in self.file_cls_attr_settings_map:
             default_value = self.default_cls_settings[pipe_attr]
             self.assertEqual(getattr(one_pipeline, pipe_attr), default_value)
             custom_value = custom_settings[settings_attr]
-            pipe_attr_lower = pipe_attr.lower().replace("default_", "")
-            self.assertEqual(getattr(another_pipeline, pipe_attr_lower), custom_value)
+            self.assertEqual(getattr(another_pipeline, pipe_ins_attr), custom_value)
 
     def test_subclass_attributes_preserved_if_no_settings(self):
         """
@@ -248,9 +247,63 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase):
         """
         pipe_cls = self._generate_fake_pipeline()
         pipe = pipe_cls.from_settings(Settings({"FILES_STORE": self.tempdir}))
-        for pipe_attr, settings_attr in self.file_cls_attr_settings_map:
-            attr_lower = pipe_attr.lower().replace("default_", "")
-            self.assertEqual(getattr(pipe, attr_lower), getattr(pipe, pipe_attr))
+        for pipe_attr, settings_attr, pipe_ins_attr in self.file_cls_attr_settings_map:
+            self.assertEqual(getattr(pipe, pipe_ins_attr), getattr(pipe, pipe_attr))
+
+    def test_subclass_attrs_preserved_custom_settings(self):
+        """
+        If file settings are defined but they are not defined for subclass class attributes
+        should be preserved.
+        """
+        pipeline_cls = self._generate_fake_pipeline()
+        settings = self._generate_fake_settings()
+        pipeline = pipeline_cls.from_settings(Settings(settings))
+        for pipe_attr, settings_attr, pipe_ins_attr in self.file_cls_attr_settings_map:
+            value = getattr(pipeline, pipe_ins_attr)
+            self.assertEqual(value, getattr(pipeline, pipe_attr))
+
+    def test_no_custom_settings_for_subclasses(self):
+        """
+        If there are no settings for subclass and no subclass attributes, pipeline should use
+        attributes of base class.
+        """
+        class UserDefinedFilesPipeline(FilesPipeline):
+            pass
+
+        user_pipeline = UserDefinedFilesPipeline.from_settings(Settings({"FILES_STORE": self.tempdir}))
+        for pipe_attr, settings_attr, pipe_ins_attr in self.file_cls_attr_settings_map:
+            # Values from settings for custom pipeline should be set on pipeline instance.
+            custom_value = self.default_cls_settings.get(pipe_attr.upper())
+            self.assertEqual(getattr(user_pipeline, pipe_ins_attr), custom_value)
+
+    def test_custom_settings_for_subclasses(self):
+        """
+        If there are custom settings for subclass and NO class attributes, pipeline should use custom
+        settings.
+        """
+        class UserDefinedFilesPipeline(FilesPipeline):
+            pass
+
+        prefix = UserDefinedFilesPipeline.__name__.upper()
+        settings = self._generate_fake_settings(prefix=prefix)
+        user_pipeline = UserDefinedFilesPipeline.from_settings(Settings(settings))
+        for pipe_attr, settings_attr, pipe_inst_attr in self.file_cls_attr_settings_map:
+            # Values from settings for custom pipeline should be set on pipeline instance.
+            custom_value = settings.get(prefix + "_" + settings_attr)
+            self.assertEqual(getattr(user_pipeline, pipe_inst_attr), custom_value)
+
+    def test_custom_settings_and_class_attrs_for_subclasses(self):
+        """
+        If there are custom settings for subclass AND class attributes
+        setting keys are preferred and override attributes.
+        """
+        pipeline_cls = self._generate_fake_pipeline()
+        prefix = pipeline_cls.__name__.upper()
+        settings = self._generate_fake_settings(prefix=prefix)
+        user_pipeline = pipeline_cls.from_settings(Settings(settings))
+        for pipe_cls_attr, settings_attr, pipe_inst_attr  in self.file_cls_attr_settings_map:
+            custom_value = settings.get(prefix + "_" + settings_attr)
+            self.assertEqual(getattr(user_pipeline, pipe_inst_attr), custom_value)
 
 
 class TestS3FilesStore(unittest.TestCase):

From 10b79c9b3ec438653b25d531c30888849e080aa5 Mon Sep 17 00:00:00 2001
From: Pawel Miech <pawelmhm@gmail.com>
Date: Wed, 15 Jun 2016 15:49:11 +0200
Subject: [PATCH 10/14] [files-pipeline] update docs with note about settings

for subclasses.
---
 docs/topics/media-pipeline.rst | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst
index 96339d03d..6cbac913c 100644
--- a/docs/topics/media-pipeline.rst
+++ b/docs/topics/media-pipeline.rst
@@ -191,9 +191,10 @@ For the Images Pipeline, set :setting:`IMAGES_URLS_FIELD` and/or
 If you need something more complex and want to override the custom pipeline
 behaviour, see :ref:`topics-media-pipeline-override`.
 
-.. note:: If you have multiple image pipelines inheriting from ImagePipeline and you want to have different settings in different pipelines
-   you can set setting keys preceded with uppercase name of your pipeline class. E.g. if your pipeline is called
-   MyPipeline and you want to have custom IMAGES_URLS_FIELD you define setting MYPIPELINE_IMAGES_URLS_FIELD.
+If you have multiple image pipelines inheriting from ImagePipeline and you want to have different settings in different pipelines
+you can set setting keys preceded with uppercase name of your pipeline class. E.g. if your pipeline is called
+MyPipeline and you want to have custom IMAGES_URLS_FIELD you define setting MYPIPELINE_IMAGES_URLS_FIELD and your custom
+settings will be used.
 
 
 Additional features
@@ -218,6 +219,14 @@ specifies the delay in number of days::
 
 The default value for both settings is 90 days.
 
+If you have pipeline that subclasses FilesPipeline and you'd like to have different setting
+for it you can set setting keys preceded by uppercase class name. E.g. given pipeline class
+called MyPipeline you can set setting key:
+
+    MYPIPELINE_FILES_EXPIRES = 180
+
+and pipeline class MyPipeline will have expiration time set to 180.
+
 .. _topics-images-thumbnails:
 
 Thumbnail generation for images

From fa4d0cdfe5df7165c220e2c17c2de14262fc713e Mon Sep 17 00:00:00 2001
From: Pawel Miech <pawelmhm@gmail.com>
Date: Mon, 20 Jun 2016 12:39:09 +0200
Subject: [PATCH 11/14] [FilesPipeline, ImagesPipeline] fix for cls attrs with
 DEFAULT prefix

some class attributes for ImagePipeline and FilesPipeline had DEFAULT prefix. These
attributes should be preserved as well, if users subclasses define values for
DEFAULT_<CLS_ATTRIBUTE_NAME> attribute this value should be preserved.
---
 scrapy/pipelines/files.py     |  8 ++++++--
 scrapy/pipelines/images.py    | 15 +++++++++++----
 tests/test_pipeline_files.py  | 29 ++++++++++++++++++++++-------
 tests/test_pipeline_images.py | 13 +++++++++++++
 4 files changed, 52 insertions(+), 13 deletions(-)

diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py
index b9c43dc3b..73eda5f34 100644
--- a/scrapy/pipelines/files.py
+++ b/scrapy/pipelines/files.py
@@ -235,11 +235,15 @@ class FilesPipeline(MediaPipeline):
         self.expires = settings.getint(
             self._key_for_pipe('FILES_EXPIRES', cls_name), self.EXPIRES
         )
+        if not hasattr(self, "FILES_URLS_FIELD"):
+            self.FILES_URLS_FIELD = self.DEFAULT_FILES_URLS_FIELD
+        if not hasattr(self, "FILES_RESULT_FIELD"):
+            self.FILES_RESULT_FIELD = self.DEFAULT_FILES_RESULT_FIELD
         self.files_urls_field = settings.get(
-            self._key_for_pipe('FILES_URLS_FIELD', cls_name), self.DEFAULT_FILES_URLS_FIELD
+            self._key_for_pipe('FILES_URLS_FIELD', cls_name), self.FILES_URLS_FIELD
         )
         self.files_result_field = settings.get(
-            self._key_for_pipe('FILES_RESULT_FIELD', cls_name), self.DEFAULT_FILES_RESULT_FIELD
+            self._key_for_pipe('FILES_RESULT_FIELD', cls_name), self.FILES_RESULT_FIELD
         )
 
         super(FilesPipeline, self).__init__(download_func=download_func)
diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py
index de616211e..73377e2c2 100644
--- a/scrapy/pipelines/images.py
+++ b/scrapy/pipelines/images.py
@@ -44,8 +44,8 @@ class ImagesPipeline(FilesPipeline):
     MIN_HEIGHT = 0
     EXPIRES = 0
     THUMBS = {}
-    IMAGES_URLS_FIELD = 'image_urls'
-    IMAGES_RESULT_FIELD = 'images'
+    DEFAULT_IMAGES_URLS_FIELD = 'image_urls'
+    DEFAULT_IMAGES_RESULT_FIELD = 'images'
 
     def __init__(self, store_uri, download_func=None, settings=None):
         super(ImagesPipeline, self).__init__(store_uri, settings=settings, download_func=download_func)
@@ -57,11 +57,18 @@ class ImagesPipeline(FilesPipeline):
         self.expires = settings.getint(
             self._key_for_pipe('IMAGES_EXPIRES', cls_name), self.EXPIRES
         )
+        if not hasattr(self, "IMAGES_RESULT_FIELD"):
+            self.IMAGES_RESULT_FIELD = self.DEFAULT_IMAGES_RESULT_FIELD
+        if not hasattr(self, "IMAGES_URLS_FIELD"):
+            self.IMAGES_URLS_FIELD = self.DEFAULT_IMAGES_URLS_FIELD
+
+        default_images_urls_field = getattr(self, "IMAGES_URLS_FIELD", "DEFAULT_IMAGES_URLS_FIELD")
         self.images_urls_field = settings.get(
-            self._key_for_pipe('IMAGES_URLS_FIELD', cls_name), self.IMAGES_URLS_FIELD
+            self._key_for_pipe('IMAGES_URLS_FIELD', cls_name), default_images_urls_field
         )
+        default_images_result_field = getattr(self, "IMAGES_RESULT_FIELD", "DEFAULT_IMAGES_RESULT_FIELD")
         self.images_result_field = settings.get(
-            self._key_for_pipe('IMAGES_RESULT_FIELD', cls_name), self.IMAGES_RESULT_FIELD
+            self._key_for_pipe('IMAGES_RESULT_FIELD', cls_name), default_images_result_field
         )
         self.min_width = settings.getint(
             self._key_for_pipe('IMAGES_MIN_WIDTH', cls_name), self.MIN_WIDTH
diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py
index fd54b7229..bda2a2199 100644
--- a/tests/test_pipeline_files.py
+++ b/tests/test_pipeline_files.py
@@ -187,13 +187,13 @@ class FilesPipelineTestCaseFields(unittest.TestCase):
 class FilesPipelineTestCaseCustomSettings(unittest.TestCase):
     default_cls_settings = {
         "EXPIRES": 90,
-        "DEFAULT_FILES_URLS_FIELD": "file_urls",
-        "DEFAULT_FILES_RESULT_FIELD": "files"
+        "FILES_URLS_FIELD": "file_urls",
+        "FILES_RESULT_FIELD": "files"
     }
     file_cls_attr_settings_map = {
         ("EXPIRES", "FILES_EXPIRES", "expires"),
-        ("DEFAULT_FILES_URLS_FIELD", "FILES_URLS_FIELD", "files_urls_field"),
-        ("DEFAULT_FILES_RESULT_FIELD", "FILES_RESULT_FIELD", "files_result_field")
+        ("FILES_URLS_FIELD", "FILES_URLS_FIELD", "files_urls_field"),
+        ("FILES_RESULT_FIELD", "FILES_RESULT_FIELD", "files_result_field")
     }
 
     def setUp(self):
@@ -221,9 +221,9 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase):
     def _generate_fake_pipeline(self):
 
         class UserDefinedFilePipeline(FilesPipeline):
-            FILES_EXPIRES = random.randint(1001, 2000)
-            DEFAULT_FILES_URLS_FIELD = "alfa"
-            DEFAULT_FILES_RESULT_FIELD = "beta"
+            EXPIRES = 1001
+            FILES_URLS_FIELD = "alfa"
+            FILES_RESULT_FIELD = "beta"
 
         return UserDefinedFilePipeline
 
@@ -239,6 +239,7 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase):
             default_value = self.default_cls_settings[pipe_attr]
             self.assertEqual(getattr(one_pipeline, pipe_attr), default_value)
             custom_value = custom_settings[settings_attr]
+            self.assertNotEqual(default_value, custom_value)
             self.assertEqual(getattr(another_pipeline, pipe_ins_attr), custom_value)
 
     def test_subclass_attributes_preserved_if_no_settings(self):
@@ -248,6 +249,8 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase):
         pipe_cls = self._generate_fake_pipeline()
         pipe = pipe_cls.from_settings(Settings({"FILES_STORE": self.tempdir}))
         for pipe_attr, settings_attr, pipe_ins_attr in self.file_cls_attr_settings_map:
+            custom_value = getattr(pipe, pipe_ins_attr)
+            self.assertNotEqual(custom_value, self.default_cls_settings[pipe_attr])
             self.assertEqual(getattr(pipe, pipe_ins_attr), getattr(pipe, pipe_attr))
 
     def test_subclass_attrs_preserved_custom_settings(self):
@@ -260,6 +263,7 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase):
         pipeline = pipeline_cls.from_settings(Settings(settings))
         for pipe_attr, settings_attr, pipe_ins_attr in self.file_cls_attr_settings_map:
             value = getattr(pipeline, pipe_ins_attr)
+            self.assertNotEqual(value, self.default_cls_settings[pipe_attr])
             self.assertEqual(value, getattr(pipeline, pipe_attr))
 
     def test_no_custom_settings_for_subclasses(self):
@@ -290,6 +294,7 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase):
         for pipe_attr, settings_attr, pipe_inst_attr in self.file_cls_attr_settings_map:
             # Values from settings for custom pipeline should be set on pipeline instance.
             custom_value = settings.get(prefix + "_" + settings_attr)
+            self.assertNotEqual(custom_value, self.default_cls_settings[pipe_attr])
             self.assertEqual(getattr(user_pipeline, pipe_inst_attr), custom_value)
 
     def test_custom_settings_and_class_attrs_for_subclasses(self):
@@ -303,8 +308,18 @@ class FilesPipelineTestCaseCustomSettings(unittest.TestCase):
         user_pipeline = pipeline_cls.from_settings(Settings(settings))
         for pipe_cls_attr, settings_attr, pipe_inst_attr  in self.file_cls_attr_settings_map:
             custom_value = settings.get(prefix + "_" + settings_attr)
+            self.assertNotEqual(custom_value, self.default_cls_settings[pipe_cls_attr])
             self.assertEqual(getattr(user_pipeline, pipe_inst_attr), custom_value)
 
+    def test_cls_attrs_with_DEFAULT_prefix(self):
+        class UserDefinedFilesPipeline(FilesPipeline):
+            DEFAULT_FILES_RESULT_FIELD = "this"
+            DEFAULT_FILES_URLS_FIELD = "that"
+
+        pipeline = UserDefinedFilesPipeline.from_settings(Settings({"FILES_STORE": self.tempdir}))
+        self.assertEqual(pipeline.files_result_field, "this")
+        self.assertEqual(pipeline.files_urls_field, "that")
+
 
 class TestS3FilesStore(unittest.TestCase):
     @defer.inlineCallbacks
diff --git a/tests/test_pipeline_images.py b/tests/test_pipeline_images.py
index a2dd5aa28..6ccd9791e 100644
--- a/tests/test_pipeline_images.py
+++ b/tests/test_pipeline_images.py
@@ -304,6 +304,7 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase):
         for pipe_attr, settings_attr in self.img_cls_attribute_names:
             # Instance attribute (lowercase) must be equal to class attribute (uppercase).
             attr_value = getattr(pipeline, pipe_attr.lower())
+            self.assertNotEqual(attr_value, self.default_pipeline_settings[pipe_attr])
             self.assertEqual(attr_value, getattr(pipeline, pipe_attr))
 
     def test_subclass_attrs_preserved_custom_settings(self):
@@ -317,6 +318,7 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase):
         for pipe_attr, settings_attr in self.img_cls_attribute_names:
             # Instance attribute (lowercase) must be equal to class attribute (uppercase).
             value = getattr(pipeline, pipe_attr.lower())
+            self.assertNotEqual(value, self.default_pipeline_settings[pipe_attr])
             self.assertEqual(value, getattr(pipeline, pipe_attr))
 
     def test_no_custom_settings_for_subclasses(self):
@@ -347,6 +349,7 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase):
         for pipe_attr, settings_attr in self.img_cls_attribute_names:
             # Values from settings for custom pipeline should be set on pipeline instance.
             custom_value = settings.get(prefix + "_" + settings_attr)
+            self.assertNotEqual(custom_value, self.default_pipeline_settings[pipe_attr])
             self.assertEqual(getattr(user_pipeline, pipe_attr.lower()), custom_value)
 
     def test_custom_settings_and_class_attrs_for_subclasses(self):
@@ -360,8 +363,18 @@ class ImagesPipelineTestCaseCustomSettings(unittest.TestCase):
         user_pipeline = pipeline_cls.from_settings(Settings(settings))
         for pipe_attr, settings_attr in self.img_cls_attribute_names:
             custom_value = settings.get(prefix + "_" + settings_attr)
+            self.assertNotEqual(custom_value, self.default_pipeline_settings[pipe_attr])
             self.assertEqual(getattr(user_pipeline, pipe_attr.lower()), custom_value)
 
+    def test_cls_attrs_with_DEFAULT_prefix(self):
+        class UserDefinedImagePipeline(ImagesPipeline):
+            DEFAULT_IMAGES_URLS_FIELD = "something"
+            DEFAULT_IMAGES_RESULT_FIELD = "something_else"
+
+        pipeline = UserDefinedImagePipeline.from_settings(Settings({"IMAGES_STORE": self.tempdir}))
+        self.assertEqual(pipeline.images_result_field, "something_else")
+        self.assertEqual(pipeline.images_urls_field, "something")
+
 
 def _create_image(format, *a, **kw):
     buf = TemporaryFile()

From c22cc1096be5aaa3f381976ba0b70a014405dc4f Mon Sep 17 00:00:00 2001
From: Pawel Miech <pawelmhm@gmail.com>
Date: Tue, 12 Jul 2016 13:58:36 +0200
Subject: [PATCH 12/14] [image_pipeline] style edits

* 80 characters line limit
* shortening some code
* removed dead code
* add doctest for _key_for_pipe function
---
 docs/topics/media-pipeline.rst | 15 ++++++++-------
 scrapy/pipelines/files.py      | 10 ++++++----
 scrapy/pipelines/images.py     | 18 ++++++++++--------
 scrapy/pipelines/media.py      | 17 ++++++++---------
 4 files changed, 32 insertions(+), 28 deletions(-)

diff --git a/docs/topics/media-pipeline.rst b/docs/topics/media-pipeline.rst
index 6cbac913c..f18789ab0 100644
--- a/docs/topics/media-pipeline.rst
+++ b/docs/topics/media-pipeline.rst
@@ -191,10 +191,11 @@ For the Images Pipeline, set :setting:`IMAGES_URLS_FIELD` and/or
 If you need something more complex and want to override the custom pipeline
 behaviour, see :ref:`topics-media-pipeline-override`.
 
-If you have multiple image pipelines inheriting from ImagePipeline and you want to have different settings in different pipelines
-you can set setting keys preceded with uppercase name of your pipeline class. E.g. if your pipeline is called
-MyPipeline and you want to have custom IMAGES_URLS_FIELD you define setting MYPIPELINE_IMAGES_URLS_FIELD and your custom
-settings will be used.
+If you have multiple image pipelines inheriting from ImagePipeline and you want
+to have different settings in different pipelines you can set setting keys
+preceded with uppercase name of your pipeline class. E.g. if your pipeline is
+called MyPipeline and you want to have custom IMAGES_URLS_FIELD you define
+setting MYPIPELINE_IMAGES_URLS_FIELD and your custom settings will be used.
 
 
 Additional features
@@ -219,9 +220,9 @@ specifies the delay in number of days::
 
 The default value for both settings is 90 days.
 
-If you have pipeline that subclasses FilesPipeline and you'd like to have different setting
-for it you can set setting keys preceded by uppercase class name. E.g. given pipeline class
-called MyPipeline you can set setting key:
+If you have pipeline that subclasses FilesPipeline and you'd like to have
+different setting for it you can set setting keys preceded by uppercase
+class name. E.g. given pipeline class called MyPipeline you can set setting key:
 
     MYPIPELINE_FILES_EXPIRES = 180
 
diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py
index 73eda5f34..8cdc548f6 100644
--- a/scrapy/pipelines/files.py
+++ b/scrapy/pipelines/files.py
@@ -3,7 +3,7 @@ Files Pipeline
 
 See documentation in topics/media-pipeline.rst
 """
-
+import functools
 import hashlib
 import os
 import os.path
@@ -232,18 +232,20 @@ class FilesPipeline(MediaPipeline):
 
         cls_name = "FilesPipeline"
         self.store = self._get_store(store_uri)
+        resolve = functools.partial(self._key_for_pipe,
+                                    base_class_name=cls_name)
         self.expires = settings.getint(
-            self._key_for_pipe('FILES_EXPIRES', cls_name), self.EXPIRES
+            resolve('FILES_EXPIRES'), self.EXPIRES
         )
         if not hasattr(self, "FILES_URLS_FIELD"):
             self.FILES_URLS_FIELD = self.DEFAULT_FILES_URLS_FIELD
         if not hasattr(self, "FILES_RESULT_FIELD"):
             self.FILES_RESULT_FIELD = self.DEFAULT_FILES_RESULT_FIELD
         self.files_urls_field = settings.get(
-            self._key_for_pipe('FILES_URLS_FIELD', cls_name), self.FILES_URLS_FIELD
+            resolve('FILES_URLS_FIELD'), self.FILES_URLS_FIELD
         )
         self.files_result_field = settings.get(
-            self._key_for_pipe('FILES_RESULT_FIELD', cls_name), self.FILES_RESULT_FIELD
+            resolve('FILES_RESULT_FIELD'), self.FILES_RESULT_FIELD
         )
 
         super(FilesPipeline, self).__init__(download_func=download_func)
diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py
index 73377e2c2..0278ec32f 100644
--- a/scrapy/pipelines/images.py
+++ b/scrapy/pipelines/images.py
@@ -3,7 +3,7 @@ Images Pipeline
 
 See documentation in topics/media-pipeline.rst
 """
-
+import functools
 import hashlib
 import six
 
@@ -62,22 +62,24 @@ class ImagesPipeline(FilesPipeline):
         if not hasattr(self, "IMAGES_URLS_FIELD"):
             self.IMAGES_URLS_FIELD = self.DEFAULT_IMAGES_URLS_FIELD
 
-        default_images_urls_field = getattr(self, "IMAGES_URLS_FIELD", "DEFAULT_IMAGES_URLS_FIELD")
+        resolve = functools.partial(self._key_for_pipe, base_class_name=cls_name)
+
         self.images_urls_field = settings.get(
-            self._key_for_pipe('IMAGES_URLS_FIELD', cls_name), default_images_urls_field
+            resolve('IMAGES_URLS_FIELD'),
+            self.IMAGES_URLS_FIELD
         )
-        default_images_result_field = getattr(self, "IMAGES_RESULT_FIELD", "DEFAULT_IMAGES_RESULT_FIELD")
         self.images_result_field = settings.get(
-            self._key_for_pipe('IMAGES_RESULT_FIELD', cls_name), default_images_result_field
+            resolve('IMAGES_RESULT_FIELD'),
+            self.IMAGES_RESULT_FIELD
         )
         self.min_width = settings.getint(
-            self._key_for_pipe('IMAGES_MIN_WIDTH', cls_name), self.MIN_WIDTH
+            resolve('IMAGES_MIN_WIDTH'), self.MIN_WIDTH
         )
         self.min_height = settings.getint(
-            self._key_for_pipe('IMAGES_MIN_HEIGHT', cls_name), self.MIN_HEIGHT
+            resolve('IMAGES_MIN_HEIGHT'), self.MIN_HEIGHT
         )
         self.thumbs = settings.get(
-            self._key_for_pipe('IMAGES_THUMBS', cls_name), self.THUMBS
+            resolve('IMAGES_THUMBS'), self.THUMBS
         )
 
     @classmethod
diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py
index 740312f8f..fe59b9f7c 100644
--- a/scrapy/pipelines/media.py
+++ b/scrapy/pipelines/media.py
@@ -28,18 +28,17 @@ class MediaPipeline(object):
         self.download_func = download_func
 
 
-    def _key_for_pipe(self, key, base_class_name):
+    def _key_for_pipe(self, key, base_class_name=None):
         """
-        Allow setting settings for user defined MediaPipelines that inherit from base.
-
-        User can define setting key:
-
-        MYPIPELINENAME_IMAGE_SETTING_NAME = <some value>
-
-        and it will override default settings and class attributes.
+        >>> result = MediaPipeline()._key_for_pipe("IMAGES")
+        >>> assert result == "IMAGES"
+        >>> class MyPipe(MediaPipeline):
+        ...     pass
+        >>> other_key = MyPipe()._key_for_pipe("IMAGES", base_class_name="MediaPipeline")
+        >>> assert other_key == "MYPIPE_IMAGES"
         """
         class_name = self.__class__.__name__
-        if class_name == base_class_name:
+        if class_name == base_class_name or not base_class_name:
             return key
         return "{}_{}".format(class_name.upper(), key)
 

From 9818c97f6e9ac721bb136e4d2a4bfe1ffc8cc047 Mon Sep 17 00:00:00 2001
From: Pawel Miech <pawelmhm@gmail.com>
Date: Tue, 12 Jul 2016 14:15:41 +0200
Subject: [PATCH 13/14] [image_pipeline] dont use assert in doctest

---
 scrapy/pipelines/media.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py
index fe59b9f7c..82b4b462e 100644
--- a/scrapy/pipelines/media.py
+++ b/scrapy/pipelines/media.py
@@ -30,12 +30,12 @@ class MediaPipeline(object):
 
     def _key_for_pipe(self, key, base_class_name=None):
         """
-        >>> result = MediaPipeline()._key_for_pipe("IMAGES")
-        >>> assert result == "IMAGES"
+        >>> MediaPipeline()._key_for_pipe("IMAGES")
+        'IMAGES'
         >>> class MyPipe(MediaPipeline):
         ...     pass
-        >>> other_key = MyPipe()._key_for_pipe("IMAGES", base_class_name="MediaPipeline")
-        >>> assert other_key == "MYPIPE_IMAGES"
+        >>> MyPipe()._key_for_pipe("IMAGES", base_class_name="MediaPipeline")
+        'MYPIPE_IMAGES'
         """
         class_name = self.__class__.__name__
         if class_name == base_class_name or not base_class_name:

From ceecf3b26c542ffcd74d6a81233d58ed54ea3268 Mon Sep 17 00:00:00 2001
From: Pawel Miech <pawelmhm@gmail.com>
Date: Wed, 13 Jul 2016 16:17:34 +0200
Subject: [PATCH 14/14] [image_pipeline] minor style tweaks

---
 scrapy/pipelines/images.py | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py
index 0278ec32f..a511887b6 100644
--- a/scrapy/pipelines/images.py
+++ b/scrapy/pipelines/images.py
@@ -48,22 +48,23 @@ class ImagesPipeline(FilesPipeline):
     DEFAULT_IMAGES_RESULT_FIELD = 'images'
 
     def __init__(self, store_uri, download_func=None, settings=None):
-        super(ImagesPipeline, self).__init__(store_uri, settings=settings, download_func=download_func)
+        super(ImagesPipeline, self).__init__(store_uri, settings=settings,
+                                             download_func=download_func)
 
         if isinstance(settings, dict) or settings is None:
             settings = Settings(settings)
 
-        cls_name = "ImagesPipeline"
+        resolve = functools.partial(self._key_for_pipe,
+                                    base_class_name="ImagesPipeline")
         self.expires = settings.getint(
-            self._key_for_pipe('IMAGES_EXPIRES', cls_name), self.EXPIRES
+            resolve("IMAGES_EXPIRES"), self.EXPIRES
         )
+
         if not hasattr(self, "IMAGES_RESULT_FIELD"):
             self.IMAGES_RESULT_FIELD = self.DEFAULT_IMAGES_RESULT_FIELD
         if not hasattr(self, "IMAGES_URLS_FIELD"):
             self.IMAGES_URLS_FIELD = self.DEFAULT_IMAGES_URLS_FIELD
 
-        resolve = functools.partial(self._key_for_pipe, base_class_name=cls_name)
-
         self.images_urls_field = settings.get(
             resolve('IMAGES_URLS_FIELD'),
             self.IMAGES_URLS_FIELD