Repository: sec-edgar/sec-edgar (python)

Issue:
The same and only file gets downloaded for all years and quarters and filing types 
**Describe the bug**
When running a sample piece of code from the official documentation (with addition of a custom `client`), the data that is downloaded is the same and the only file that gets downloaded for all years, quarters and filing types. See screenshot

**To Reproduce**
Steps to reproduce the behavior:
1. Run the code snippet below

```
from datetime import date
from secedgar import filings

a = NetworkClient(rate_limit=1,user_agent="Mozilla")
limit_to_form13 = lambda f: f.form_type in ("13F-HR", "13F-HR/A") 

daily_filings_limited = filings(start_date=date(1993, 1 ,1),
                                end_date=date(2021, 3, 31),
                                client=a,
                                entry_filter=limit_to_form13)

daily_filings_limited.save("./data_13F")
```

**Expected behavior**
All 13F filings being downloaded for all companies and all years

**Screenshots**
![image](https://user-images.githubusercontent.com/9093546/137359490-40cec6e0-b0ee-4a98-b429-1a91f29d60b5.png)


**Desktop (please complete the following information):**
Windows 11
v0.4.0-alpha.2

**Additional context**
Add any other context about the problem here.


Pull request:
FIX: Fix caching issue and simplify combo code


- [x] closes #243
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] added entry to docs/whatsnew latest version

Patch:
diff --git a/docs/source/whatsnew/v0.4.0.rst b/docs/source/whatsnew/v0.4.0.rst
index 84a8249..52354c6 100644
--- a/docs/source/whatsnew/v0.4.0.rst
+++ b/docs/source/whatsnew/v0.4.0.rst
@@ -17,6 +17,8 @@ Many breaking changes, as this update does a lot of restructuring.
 - Fix issue where daily CLI command would not work due to ``date_cleanup`` returning ``datetime.datetime`` instead of ``datetime.date``
 - Fix issue where ``CompanyFilings`` includes links that are not exact matches to ``filing_type``.
 - Add NSAR filing types. Thanks to mjkoo91!
+- Get rid of caching behavior that was causing issues when downloading filings.
+- Simplify ``ComboFilings`` logic for getting quarterly and daily dates.
 - Fix issue where ``get_cik_map`` fails due to the SEC providing ``None`` for title or ticker.
 
 Contributors
diff --git a/secedgar/core/_index.py b/secedgar/core/_index.py
index be2cdcf..71cd44a 100644
--- a/secedgar/core/_index.py
+++ b/secedgar/core/_index.py
@@ -142,43 +142,39 @@ class IndexFilings(AbstractFiling):
                                       .format(filename=self.idx_filename))
         return self._master_idx_file
 
-    def get_filings_dict(self, update_cache=False, **kwargs):
+    def get_filings_dict(self, **kwargs):
         """Get all filings inside an idx file.
 
         Args:
-            update_cache (bool, optional): Whether filings dict should be
-                updated on each method call. Defaults to False.
-
             kwargs: Any kwargs to pass to _get_master_idx_file. See
                 ``secedgar.core.daily.DailyFilings._get_master_idx_file``.
         """
-        if self._filings_dict is None or update_cache:
-            idx_file = self._get_master_idx_file(**kwargs)
-            # Will have CIK as keys and list of FilingEntry namedtuples as values
-            self._filings_dict = {}
-            FilingEntry = namedtuple("FilingEntry", [
-                "cik", "company_name", "form_type", "date_filed", "file_name",
-                "path", "num_previously_valid"
-            ])
-            # idx file will have lines of the form CIK|Company Name|Form Type|Date Filed|File Name
-            current_count = 0
-            entries = re.findall(r'^[0-9]+[|].+[|].+[|][0-9\-]+[|].+$',
-                                 idx_file, re.MULTILINE)
-            for entry in entries:
-                fields = entry.split("|")
-                path = "Archives/{file_name}".format(file_name=fields[-1])
-                entry = FilingEntry(*fields,
-                                    path=path,
-                                    num_previously_valid=current_count)
-                if self.entry_filter is not None and not self.entry_filter(
-                        entry):
-                    continue
-                current_count += 1
-                # Add new filing entry to CIK's list
-                if entry.cik in self._filings_dict:
-                    self._filings_dict[entry.cik].append(entry)
-                else:
-                    self._filings_dict[entry.cik] = [entry]
+        idx_file = self._get_master_idx_file(**kwargs)
+        # Will have CIK as keys and list of FilingEntry namedtuples as values
+        self._filings_dict = {}
+        FilingEntry = namedtuple("FilingEntry", [
+            "cik", "company_name", "form_type", "date_filed", "file_name",
+            "path", "num_previously_valid"
+        ])
+        # idx file will have lines of the form CIK|Company Name|Form Type|Date Filed|File Name
+        current_count = 0
+        entries = re.findall(r'^[0-9]+[|].+[|].+[|][0-9\-]+[|].+$',
+                             idx_file, re.MULTILINE)
+        for entry in entries:
+            fields = entry.split("|")
+            path = "Archives/{file_name}".format(file_name=fields[-1])
+            entry = FilingEntry(*fields,
+                                path=path,
+                                num_previously_valid=current_count)
+            if self.entry_filter is not None and not self.entry_filter(
+                    entry):
+                continue
+            current_count += 1
+            # Add new filing entry to CIK's list
+            if entry.cik in self._filings_dict:
+                self._filings_dict[entry.cik].append(entry)
+            else:
+                self._filings_dict[entry.cik] = [entry]
         return self._filings_dict
 
     def get_urls(self):
@@ -189,13 +185,12 @@ class IndexFilings(AbstractFiling):
         Returns:
             urls (list of str): List of all URLs to get.
         """
-        if not self._urls:
-            filings_dict = self.get_filings_dict()
-            self._urls = {
-                company:
-                [self.client._prepare_query(entry.path) for entry in entries]
-                for company, entries in filings_dict.items()
-            }
+        filings_dict = self.get_filings_dict()
+        self._urls = {
+            company:
+            [self.client._prepare_query(entry.path) for entry in entries]
+            for company, entries in filings_dict.items()
+        }
         return self._urls
 
     @staticmethod
diff --git a/secedgar/core/combo.py b/secedgar/core/combo.py
index 604ba04..788e310 100644
--- a/secedgar/core/combo.py
+++ b/secedgar/core/combo.py
@@ -2,12 +2,32 @@ from datetime import date, timedelta
 from functools import reduce
 from typing import Union
 
+from secedgar.client import NetworkClient
 from secedgar.core.daily import DailyFilings
 from secedgar.core.quarterly import QuarterlyFilings
 from secedgar.exceptions import EDGARQueryError, NoFilingsError
 from secedgar.utils import add_quarter, get_month, get_quarter
 
 
+def fill_days(start, end, include_start=False, include_end=False):
+    """Get dates for days in between start and end date.
+
+    Args:
+        start (``datetime.date``): Start date.
+        end (``datetime.date``): End date.
+        include_start (bool, optional): Whether or not to include start date in range.
+            Defaults to False.
+        include_end (bool, optional): Whether or not to include end date in range.
+            Defaults to False.
+
+    Returns:
+        list of ``datetime.date``: List of dates between ``start`` and ``end``.
+    """
+    start_range = 0 if include_start else 1
+    end_range = (end - start).days + 1 if include_end else (end - start).days
+    return [start + timedelta(days=d) for d in range(start_range, end_range)]
+
+
 class ComboFilings:
     """Class for retrieving all filings between specified dates.
 
@@ -64,92 +84,154 @@ class ComboFilings:
         self.entry_filter = entry_filter
         self.start_date = start_date
         self.end_date = end_date
-        self.user_agent = user_agent
-        self.quarterly = QuarterlyFilings(year=self.start_date.year,
-                                          quarter=get_quarter(self.start_date),
-                                          user_agent=user_agent,
-                                          client=client,
-                                          entry_filter=self.entry_filter,
-                                          **kwargs)
-        self.daily = DailyFilings(date=self.start_date,
-                                  user_agent=user_agent,
-                                  client=client,
-                                  entry_filter=self.entry_filter,
-                                  **kwargs)
-        self.balancing_point = balancing_point
-        self._recompute()
-
-    def _recompute(self):
-        """Recompute the best list of quarters and days to use based on the start and end date."""
+        self._client = client or NetworkClient(user_agent=user_agent, **kwargs)
+        self._balancing_point = balancing_point
+
+    @property
+    def entry_filter(self):
+        """A boolean function to be tested on each listing entry.
+
+        This is tested regardless of download method.
+        """
+        return self._entry_filter
+
+    @entry_filter.setter
+    def entry_filter(self, fn):
+        if callable(fn):
+            self._entry_filter = fn
+        else:
+            raise ValueError('entry_filter must be a function or lambda.')
+
+    @property
+    def client(self):
+        """``secedgar.client.NetworkClient``: Client to use to make requests."""
+        return self._client
+
+    @property
+    def start_date(self):
+        """Union([datetime.date]): Date before which no filings fetched."""
+        return self._start_date
+
+    @start_date.setter
+    def start_date(self, val):
+        if val:
+            self._start_date = val
+        else:
+            self._start_date = None
+
+    @property
+    def end_date(self):
+        """Union([datetime.date]): Date after which no filings fetched."""
+        return self._end_date
+
+    @end_date.setter
+    def end_date(self, val):
+        self._end_date = val
+
+    @property
+    def balancing_point(self):
+        """int: Point after which to use ``QuarterlyFilings`` with ``entry_filter`` to get data."""
+        return self._balancing_point
+
+    def _get_quarterly_daily_date_lists(self):
+        """Break down date range into combination of quarters and single dates.
+
+        Returns:
+            tuple of lists: Quarterly date list and daily date list.
+        """
+        # Initialize quarter and date lists
         current_date = self.start_date
-        self.quarterly_date_list = []
-        self.daily_date_list = []
+        quarterly_date_list = []
+        daily_date_list = []
+
         while current_date <= self.end_date:
             current_quarter = get_quarter(current_date)
             current_year = current_date.year
             next_year, next_quarter = add_quarter(current_year, current_quarter)
-            next_start_quarter_date = date(next_year, get_month(next_quarter),
-                                           1)
+            next_start_quarter_date = date(next_year, get_month(next_quarter), 1)
 
             days_till_next_quarter = (next_start_quarter_date -
                                       current_date).days
             days_till_end = (self.end_date - current_date).days
+
+            # If there are more days until the end date than there are
+            # in the quarter, add
             if days_till_next_quarter <= days_till_end:
                 current_start_quarter_date = date(current_year,
                                                   get_month(current_quarter), 1)
                 if current_start_quarter_date == current_date:
-                    self.quarterly_date_list.append(
+                    quarterly_date_list.append(
                         (current_year, current_quarter, lambda x: True))
                     current_date = next_start_quarter_date
                 elif days_till_next_quarter > self.balancing_point:
-                    self.quarterly_date_list.append(
+                    quarterly_date_list.append(
                         (current_year, current_quarter,
                          lambda x: date(x['date_filed']) >= self.start_date))
                     current_date = next_start_quarter_date
                 else:
-                    while current_date < next_start_quarter_date:
-                        self.daily_date_list.append(current_date)
-                        current_date += timedelta(days=1)
+                    daily_date_list.extend(fill_days(start=current_date,
+                                                     end=next_start_quarter_date,
+                                                     include_start=True,
+                                                     include_end=False))
+                    current_date = next_start_quarter_date
             else:
                 if days_till_end > self.balancing_point:
                     if days_till_next_quarter - 1 == days_till_end:
-                        self.quarterly_date_list.append(
+                        quarterly_date_list.append(
                             (current_year, current_quarter, lambda x: True))
                         current_date = next_start_quarter_date
                     else:
-                        self.quarterly_date_list.append(
+                        quarterly_date_list.append(
                             (current_year, current_quarter,
                              lambda x: date(x['date_filed']) <= self.end_date))
                         current_date = self.end_date
                 else:
-                    while current_date <= self.end_date:
-                        self.daily_date_list.append(current_date)
-                        current_date += timedelta(days=1)
+                    daily_date_list.extend(fill_days(start=current_date,
+                                                     end=self.end_date,
+                                                     include_start=True,
+                                                     include_end=True))
+                    break
+        return quarterly_date_list, daily_date_list
+
+    @property
+    def quarterly_date_list(self):
+        """List of tuples: List of tuples with year, quarter, and ``entry_filter`` to use."""
+        return self._get_quarterly_daily_date_lists()[0]  # 0 = quarterly
+
+    @property
+    def daily_date_list(self):
+        """List of ``datetime.date``: List of dates for which to fetch daily data."""
+        return self._get_quarterly_daily_date_lists()[1]  # 1 = daily
 
     def get_urls(self):
         """Get all urls between ``start_date`` and ``end_date``."""
         # Use functools.reduce for speed
         # see https://stackoverflow.com/questions/10461531/merge-and-sum-of-two-dictionaries
-        def reducer(accumulator, dictionary):
+        def _reducer(accumulator, dictionary):
             for key, value in dictionary.items():
                 accumulator[key] = accumulator.get(key, []) + value
             return accumulator
 
         list_of_dicts = []
         for (year, quarter, f) in self.quarterly_date_list:
-            self.quarterly.year = year
-            self.quarterly.quarter = quarter
-            self.quarterly.entry_filter = lambda x: f(x) and self.entry_filter(x)
-            list_of_dicts.append(self.quarterly.get_urls())
+            q = QuarterlyFilings(year=year,
+                                 quarter=quarter,
+                                 user_agent=self.user_agent,
+                                 client=self.client,
+                                 entry_filter=lambda x: f(x) and self.entry_filter(x))
+            list_of_dicts.append(q.get_urls())
 
-        for d in self.daily_date_list:
-            self.daily.date = d
+        for _date in self.daily_date_list:
+            d = DailyFilings(date=_date,
+                             user_agent=self.user_agent,
+                             client=self.client,
+                             entry_filter=self.entry_filter)
             try:
-                list_of_dicts.append(self.daily.get_urls())
-            except EDGARQueryError:
-                pass
+                list_of_dicts.append(d.get_urls())
+            except EDGARQueryError:  # continue if no URLs available for given day
+                continue
 
-        complete_dictionary = reduce(reducer, list_of_dicts, {})
+        complete_dictionary = reduce(_reducer, list_of_dicts, {})
         return complete_dictionary
 
     def save(self,
@@ -173,22 +255,28 @@ class ComboFilings:
             daily_date_format (str, optional): Format string to use for the `{date}` pattern.
                 Defaults to "%Y%m%d".
         """
+        # Go through all quarters and dates and save filings using appropriate class
         for (year, quarter, f) in self.quarterly_date_list:
-            self.quarterly.year = year
-            self.quarterly.quarter = quarter
-            self.quarterly.entry_filter = lambda x: f(x) and self.entry_filter(x)
-            self.quarterly.save(directory=directory,
-                                dir_pattern=dir_pattern,
-                                file_pattern=file_pattern,
-                                download_all=download_all)
-
-        for d in self.daily_date_list:
-            self.daily.date = d
+            q = QuarterlyFilings(year=year,
+                                 quarter=quarter,
+                                 user_agent=self.client.user_agent,
+                                 client=self.client,
+                                 entry_filter=lambda x: f(x) and self.entry_filter(x))
+            q.save(directory=directory,
+                   dir_pattern=dir_pattern,
+                   file_pattern=file_pattern,
+                   download_all=download_all)
+
+        for date_ in self.daily_date_list:
+            d = DailyFilings(date=date_,
+                             user_agent=self.client.user_agent,
+                             client=self.client,
+                             entry_filter=self.entry_filter)
             try:
-                self.daily.save(directory=directory,
-                                dir_pattern=dir_pattern,
-                                file_pattern=file_pattern,
-                                download_all=download_all,
-                                date_format=daily_date_format)
-            except (EDGARQueryError, NoFilingsError):
-                pass
+                d.save(directory=directory,
+                       dir_pattern=dir_pattern,
+                       file_pattern=file_pattern,
+                       download_all=download_all,
+                       date_format=daily_date_format)
+            except (EDGARQueryError, NoFilingsError):  # continue if no filings for given day
+                continue


Test patch:
diff --git a/secedgar/tests/core/test_combo.py b/secedgar/tests/core/test_combo.py
index 55ea6a2..bab68c7 100644
--- a/secedgar/tests/core/test_combo.py
+++ b/secedgar/tests/core/test_combo.py
@@ -1,8 +1,9 @@
 from datetime import date
 
 import pytest
+
 from secedgar.client import NetworkClient
-from secedgar.core.combo import ComboFilings
+from secedgar.core.combo import ComboFilings, fill_days
 
 
 def lambda_matches(a, b):
@@ -18,6 +19,22 @@ def quarterly_list_matches(l1, l2):
 
 
 class TestComboFilings:
+    @pytest.mark.parametrize(
+        "include_start,include_end,expected",
+        [
+            (True, True, [date(2020, 1, i) for i in (1, 2, 3)]),
+            (True, False, [date(2020, 1, i) for i in (1, 2)]),
+            (False, False, [date(2020, 1, 2)]),
+            (False, True, [date(2020, 1, i) for i in (2, 3)]),
+        ]
+    )
+    def test_fill_days(self, include_start, include_end, expected):
+        result = fill_days(start=date(2020, 1, 1),
+                           end=date(2020, 1, 3),
+                           include_start=include_start,
+                           include_end=include_end)
+        assert result == expected
+
     def test_user_agent_client_none(self):
         with pytest.raises(TypeError):
             _ = ComboFilings(start_date=date(2020, 1, 1),
@@ -30,8 +47,7 @@ class TestComboFilings:
         combo = ComboFilings(start_date=date(2020, 1, 1),
                              end_date=date(2020, 12, 31),
                              client=client)
-        assert combo.quarterly.client == client
-        assert combo.daily.client == client
+        assert combo.client == client
 
     def test_combo_quarterly_only_one_year(self, mock_user_agent):
         combo = ComboFilings(start_date=date(2020, 1, 1),
@@ -102,9 +118,60 @@ class TestComboFilings:
         assert [str(s) for s in combo.daily_date_list] == daily_expected
         assert quarterly_list_matches(combo.quarterly_date_list, quarterly_expected)
 
-    def test_user_agent_passed_to_client(self, mock_user_agent):
+    def test_properties_on_init(self, mock_user_agent):
+        start = date(2020, 1, 1)
+        end = date(2020, 5, 30)
+        bp = 25
+        combo = ComboFilings(start_date=start,
+                             end_date=end,
+                             user_agent=mock_user_agent,
+                             balancing_point=bp)
+
+        assert combo.start_date == start
+        assert combo.end_date == end
+        assert combo.balancing_point == bp
+        assert combo.client.user_agent == mock_user_agent
+
+    @pytest.mark.parametrize(
+        "bad_entry_filter",
+        [
+            None,
+            [],
+            (),
+            0,
+            ""]
+    )
+    def test_bad_entry_filter(self, bad_entry_filter):
+        with pytest.raises(ValueError):
+            _ = ComboFilings(start_date=date(2020, 1, 1),
+                             end_date=date(2020, 5, 30),
+                             entry_filter=bad_entry_filter)
+
+    @pytest.mark.parametrize(
+        "good_entry_filter",
+        [
+            lambda x: True,
+            lambda x: False,
+            lambda f: f.form_type.lower() == "10-k"
+        ]
+    )
+    def test_good_entry_filter(self, good_entry_filter, mock_user_agent):
+        combo = ComboFilings(date(2020, 1, 1),
+                             date(2020, 5, 30),
+                             entry_filter=good_entry_filter,
+                             user_agent=mock_user_agent)
+        assert combo.entry_filter == good_entry_filter
+
+    def test_client_read_only(self, mock_user_agent):
+        combo = ComboFilings(start_date=date(2020, 1, 1),
+                             end_date=date(2020, 1, 3),
+                             user_agent=mock_user_agent)
+        with pytest.raises(AttributeError):
+            combo.client = None
+
+    def test_balancing_point_read_only(self, mock_user_agent):
         combo = ComboFilings(start_date=date(2020, 1, 1),
-                             end_date=date(2021, 1, 1),
+                             end_date=date(2020, 1, 3),
                              user_agent=mock_user_agent)
-        assert combo.quarterly.client.user_agent == mock_user_agent
-        assert combo.daily.client.user_agent == mock_user_agent
+        with pytest.raises(AttributeError):
+            combo.balancing_point = 20