Bugfix for attribute inheritance in ShardedDatasetReader (#4830)

* Bugfix for issue #4825

Implemented automatic parameters inheritance from base_reader in ShardedDatasetReader.

* Updated CHANGELOG.md

* Constrained the parameters that can be inherited from the base reader

* Simplified logic for setting only `lazy` parameters

* Update CHANGELOG.md

Co-authored-by: Evan Pete Walsh <[email protected]>

Author: aleSuglia

CHANGELOG.md

@@ -14,9 +14,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
-- Fixed issue with GradientDescentTrainer when constructed with validation_data_loader==None and learning_rate_scheduler!=None.
+- Fixed issue with `GradientDescentTrainer` when constructed with `validation_data_loader=None` and `learning_rate_scheduler!=None`.
 - Fixed a bug when removing all handlers in root logger.
-
+- `ShardedDatasetReader` now inherits parameters from `base_reader` when required.
 
 ## [v1.2.2](https://github.com/allenai/allennlp/releases/tag/v1.2.2) - 2020-11-17
 

allennlp/data/dataset_readers/sharded_dataset_reader.py

@@ -30,13 +30,23 @@ class ShardedDatasetReader(DatasetReader):
 
     Registered as a `DatasetReader` with name "sharded".
 
+    This class accepts all additional parameters of any `DatasetReader` class via `**kwargs`.
+    We give priority to the values set in the constructor for the instance of this class.
+    Optionally, we will automatically inherit attributes from the `base_reader` when required.
+
     # Parameters
 
     base_reader : `DatasetReader`
         Reader with a read method that accepts a single file.
     """
 
     def __init__(self, base_reader: DatasetReader, **kwargs) -> None:
+        # ShardedDatasetReader is a wrapper for the original base_reader so some of the parameters like 'lazy'
+        # can be safely inherited. However, ShardedDatasetReader is a class instance of a DatasetReader as well.
+        # So we give priority to the parameters for the current instance stored in 'kwargs'.
+        # If not present, we check the ones in the base reader
+        kwargs["lazy"] = kwargs.get("lazy", base_reader.lazy)
+
         super().__init__(manual_distributed_sharding=True, **kwargs)
 
         if util.is_distributed():

tests/data/dataset_readers/sharded_dataset_reader_test.py

@@ -1,7 +1,7 @@
-from collections import Counter
 import glob
 import os
 import tarfile
+from collections import Counter
 from typing import Tuple
 
 import pytest
@@ -91,3 +91,20 @@ def test_sharded_read_glob(self):
 
     def test_sharded_read_archive(self):
         self.read_and_check_instances(str(self.archive_filename))
+
+    def test_attributes_inheritance(self):
+        # current reader has lazy set to true
+        base_reader = SequenceTaggingDatasetReader(lazy=True)
+        reader = ShardedDatasetReader(base_reader=base_reader)
+
+        assert (
+            reader.lazy
+        ), "The ShardedDatasetReader didn't inherit the 'lazy' attribute from base_reader"
+
+    def test_set_attributes_main(self):
+        base_reader = SequenceTaggingDatasetReader(lazy=True)
+        reader = ShardedDatasetReader(base_reader=base_reader, lazy=False)
+
+        assert (
+            not reader.lazy
+        ), "The ShardedDatasetReader inherited the 'lazy' attribute from base_reader. It should be False"