Repository: caikit/caikit (python)

Issue:
Ignore extra fields in training data
## Description

The request is for training data streams to allow extra fields that aren't in the training data model to be passed along from files. We can consider allowing extra fields when loading `DataStreamSource`s from files without making yet another config flag to turn it off.

## Acceptance Criteria

<!-- Remove any that don't apply -->

- [ ] Unit tests cover new/changed code
- [ ] Examples build against new/changed code
- [ ] READMEs are updated
- [ ] Type of [semantic version](https://semver.org/) change is identified


Pull request:
:sparkles: Allow for extra fields in data sources
<!--  Thanks for sending a pull request!  Here are some tips for you:
1. Make sure to read the Contributing Guide before submitting your PR: https://github.com/caikit/caikit/blob/main/CONTRIBUTING.md
2. If this PR closes another issue, add 'closes #<issue number>' somewhere in the PR summary. GitHub will automatically close that issue when this PR gets merged. Alternatively, adding 'refs #<issue number>' will not close the issue, but help provide the reviewer more context.-->

**What this PR does / why we need it**:
Closes #513 
- Users want to be able to provide training data files with extra fields that might not be strictly necessary for the training process without erroring
- Re-orders the methods tried for parsing data source files without extensions to allow multiparts to be tried first, otherwise they may be incorrectly attempted to be parsed by other methods - note: this does not address "missing" data model fields being represented as null since [protobuf is used to parse json](https://github.com/caikit/caikit/blob/b5c12d7bd93db40b6f53e9cef6e90be8113fe538/caikit/core/data_model/base.py#L765-L767)
- Removes some unneeded test fixture parameters

**Special notes for your reviewer**:

**If applicable**:
- [ ] this PR contains documentation
- [x] this PR contains unit tests
- [ ] this PR has been tested for backwards compatibility


Patch:
diff --git a/caikit/core/data_model/base.py b/caikit/core/data_model/base.py
index 7ec37af3..1715825b 100644
--- a/caikit/core/data_model/base.py
+++ b/caikit/core/data_model/base.py
@@ -742,13 +742,14 @@ class DataBase(metaclass=_DataBaseMetaClass):
         return cls(**kwargs)
 
     @classmethod
-    def from_json(cls, json_str):
+    def from_json(cls, json_str, ignore_unknown_fields=False):
         """Build a DataBase from a given JSON string. Use google's protobufs.json_format for
         deserialization
 
         Args:
             json_str (str or dict): A stringified JSON specification/dict of the
                 data_model
+            ignore_unknown_fields (bool): If True, ignores unknown JSON fields
 
         Returns:
             caikit.core.data_model.DataBase: A DataBase object.
@@ -763,7 +764,9 @@ class DataBase(metaclass=_DataBaseMetaClass):
         try:
             # Parse given JSON into google.protobufs.pyext.cpp_message.GeneratedProtocolMessageType
             parsed_proto = json_format.Parse(
-                json_str, cls.get_proto_class()(), ignore_unknown_fields=False
+                json_str,
+                cls.get_proto_class()(),
+                ignore_unknown_fields=ignore_unknown_fields,
             )
 
             # Use from_proto to return the DataBase object from the parsed proto
diff --git a/caikit/runtime/service_generation/data_stream_source.py b/caikit/runtime/service_generation/data_stream_source.py
index 1849ac1e..d203f744 100644
--- a/caikit/runtime/service_generation/data_stream_source.py
+++ b/caikit/runtime/service_generation/data_stream_source.py
@@ -196,14 +196,23 @@ class DataStreamSourceBase(DataStream):
     @classmethod
     def _load_from_file_without_extension(cls, fname) -> DataStream:
         """Similar to _create_data_stream_from_file, but we don't have a file extension to work
-        with. Attempt to create a data stream using one of a few well-known formats"""
+        with. Attempt to create a data stream using one of a few well-known formats.
+        🌶🌶🌶️ on ordering here:
+        File formats are loosely arranged in order of least-to-most-sketchy format validation.
+        1. .json/.jsonl are pretty straightforward
+        2. multipart files are a little iffy- the content-type header line can be omitted, in
+            which case we check for a `--` string and roll our own boundary parser. This could
+            cause problems in the future for multi-yaml files that begin with `---`
+        3. CSV support simply assumes the first line of the file has the column headers, and may
+            confidently return a stream even if that's not the case.
+        """
         full_fname = cls._get_resolved_source_path(fname)
         log.debug3("Attempting to guess file type for file: %s", full_fname)
         for factory_method in (
             DataStream.from_json_array,
             DataStream.from_jsonl,
-            DataStream.from_header_csv,
             DataStream.from_multipart_file,
+            DataStream.from_header_csv,
         ):
             try:
                 stream = factory_method(full_fname).map(cls._to_element_type)
@@ -231,7 +240,9 @@ class DataStreamSourceBase(DataStream):
         to the underlying data objects
         """
         if issubclass(cls.ELEMENT_TYPE, DataBase):
-            return cls.ELEMENT_TYPE.from_json(raw_element)
+            # To allow for extra fields (e.g. in training data) that may not
+            # be needed by the data objects, we ignore unknown fields
+            return cls.ELEMENT_TYPE.from_json(raw_element, ignore_unknown_fields=True)
         return raw_element
 
     @staticmethod


Test patch:
diff --git a/tests/core/data_model/test_dataobject.py b/tests/core/data_model/test_dataobject.py
index b9e738cf..644cffd3 100644
--- a/tests/core/data_model/test_dataobject.py
+++ b/tests/core/data_model/test_dataobject.py
@@ -1340,3 +1340,27 @@ def test_dataobject_custom_repr():
 
     inst = Foo(1)
     assert repr(inst) == custom_repr
+
+
+def test_dataobject_from_json_default_errors_on_extra_fields():
+    """Without ignore_unknown_fields, error on
+    building data object from extra fields"""
+
+    @dataobject
+    class Moo(DataObjectBase):
+        foo: int
+
+    with pytest.raises(ValueError):
+        Moo.from_json({"foo": 2, "boo": True})
+
+
+def test_dataobject_from_json_ignore_unknown_fields():
+    """With ignore_unknown_fields, ignore extra fields"""
+
+    @dataobject
+    class Moo(DataObjectBase):
+        foo: int
+
+    m1 = Moo.from_json({"foo": 2, "boo": True}, ignore_unknown_fields=True)
+    assert isinstance(m1, Moo)
+    assert m1.foo == 2
diff --git a/tests/runtime/service_generation/test_data_stream_source.py b/tests/runtime/service_generation/test_data_stream_source.py
index 615e90e8..15bae671 100644
--- a/tests/runtime/service_generation/test_data_stream_source.py
+++ b/tests/runtime/service_generation/test_data_stream_source.py
@@ -1,6 +1,4 @@
 # Standard
-from dataclasses import dataclass
-from typing import List
 import json
 import os
 import pickle
@@ -67,12 +65,12 @@ def test_multiple_make_data_stream_source():
     assert stream_type.from_proto(proto_repr).to_proto() == proto_repr
 
 
-def test_data_model_element_type(sample_train_service):
+def test_data_model_element_type():
     stream_type = caikit.interfaces.common.data_model.DataStreamSourceSampleTrainingType
     assert isinstance(stream_type._to_element_type({"number": 1}), SampleTrainingType)
 
 
-def test_primitive_element_type(sample_train_service):
+def test_primitive_element_type():
     stream_type = caikit.interfaces.common.data_model.DataStreamSourceInt
     assert isinstance(stream_type._to_element_type(1), int)
 
@@ -83,7 +81,7 @@ def test_make_data_stream_source_types():
     assert issubclass(make_data_stream_source(str), DataStreamSourceBase)
 
 
-def test_make_data_stream_source_empty(sample_train_service):
+def test_make_data_stream_source_empty():
     stream_type = caikit.interfaces.common.data_model.DataStreamSourceSampleTrainingType
     ds = stream_type()
     assert isinstance(ds, DataStreamSourceBase)
@@ -205,7 +203,7 @@ def test_data_stream_source_base_path():
 #################
 
 
-def test_make_data_stream_source_jsondata(sample_train_service):
+def test_make_data_stream_source_jsondata():
     stream_type = caikit.interfaces.common.data_model.DataStreamSourceSampleTrainingType
     ds = stream_type(
         jsondata=stream_type.JsonData(
@@ -220,7 +218,7 @@ def test_make_data_stream_source_jsondata(sample_train_service):
     validate_data_stream(data_stream, 2, SampleTrainingType)
 
 
-def test_make_data_stream_source_jsondata_other_task(sample_train_service):
+def test_make_data_stream_source_jsondata_other_task():
     stream_type = caikit.interfaces.common.data_model.DataStreamSourceInt
     ds = stream_type(jsondata=stream_type.JsonData(data=[1]))
     assert isinstance(ds, DataStreamSourceBase)
@@ -236,7 +234,7 @@ def test_make_data_stream_source_jsondata_other_task(sample_train_service):
 #################
 
 
-def test_make_data_stream_source_jsonfile(sample_train_service, sample_json_file):
+def test_make_data_stream_source_jsonfile(sample_json_file):
     stream_type = caikit.interfaces.common.data_model.DataStreamSourceSampleTrainingType
     ds = stream_type(file=File(filename=sample_json_file))
     assert isinstance(ds, DataStreamSourceBase)
@@ -247,7 +245,7 @@ def test_make_data_stream_source_jsonfile(sample_train_service, sample_json_file
     validate_data_stream(data_stream, 2, SampleTrainingType)
 
 
-def test_make_data_stream_source_csvfile(sample_train_service, sample_csv_file):
+def test_make_data_stream_source_csvfile(sample_csv_file):
     stream_type = caikit.interfaces.common.data_model.DataStreamSourceSampleTrainingType
     ds = stream_type(file=File(filename=sample_csv_file))
     assert isinstance(ds, DataStreamSourceBase)
@@ -258,7 +256,7 @@ def test_make_data_stream_source_csvfile(sample_train_service, sample_csv_file):
     validate_data_stream(data_stream, 3, SampleTrainingType)
 
 
-def test_make_data_stream_source_jsonlfile(sample_train_service, sample_jsonl_file):
+def test_make_data_stream_source_jsonlfile(sample_jsonl_file):
     stream_type = caikit.interfaces.common.data_model.DataStreamSourceSampleTrainingType
     ds = stream_type(file=File(filename=sample_jsonl_file))
     assert isinstance(ds, DataStreamSourceBase)
@@ -269,8 +267,29 @@ def test_make_data_stream_source_jsonlfile(sample_train_service, sample_jsonl_fi
     validate_data_stream(data_stream, 4, SampleTrainingType)
 
 
+def test_make_data_stream_source_jsonlfile_extra_fields(tmp_path):
+    """Test that extra fields that may be present in files are just ignored"""
+    stream_type = caikit.interfaces.common.data_model.DataStreamSourceSampleTrainingType
+    file_path = os.path.join(str(tmp_path), "extra_fields.jsonl")
+    with open(file_path, "w") as fp:
+        fp.write(
+            """
+        {"number": 2, "label": "bar", "description": "bar"}
+        {"number": 3, "label": "foo", "description": "foo"}
+        """
+        )
+
+    ds = stream_type(file=File(filename=file_path))
+    assert isinstance(ds, DataStreamSourceBase)
+
+    data_stream = ds.to_data_stream()
+    assert isinstance(data_stream, DataStream)
+
+    validate_data_stream(data_stream, 2, SampleTrainingType)
+
+
 def test_make_data_stream_source_from_file_with_no_extension(
-    sample_train_service, sample_json_file, sample_jsonl_file, sample_csv_file, tmp_path
+    sample_json_file, sample_jsonl_file, sample_csv_file, tmp_path
 ):
     stream_type = caikit.interfaces.common.data_model.DataStreamSourceSampleTrainingType
     for file in [sample_json_file, sample_jsonl_file, sample_csv_file]:
@@ -287,7 +306,6 @@ def test_make_data_stream_source_from_file_with_no_extension(
 
 
 def test_make_data_stream_source_from_multipart_formdata_file(
-    sample_train_service,
     sample_multipart_json,
     sample_multipart_csv,
     sample_multipart_json_with_content_header,
@@ -319,9 +337,7 @@ def test_make_data_stream_source_from_multipart_formdata_file(
 #################
 
 
-def test_make_data_stream_source_list_of_json_files(
-    sample_train_service, sample_json_file
-):
+def test_make_data_stream_source_list_of_json_files(sample_json_file):
     stream_type = caikit.interfaces.common.data_model.DataStreamSourceSampleTrainingType
     # does NOT work with sample_json_collection as each file needs to have data in array
     ds = stream_type(
@@ -335,9 +351,7 @@ def test_make_data_stream_source_list_of_json_files(
     validate_data_stream(data_stream, 4, SampleTrainingType)
 
 
-def test_make_data_stream_source_list_of_csv_files(
-    sample_train_service, sample_csv_collection
-):
+def test_make_data_stream_source_list_of_csv_files(sample_csv_collection):
     stream_type = caikit.interfaces.common.data_model.DataStreamSourceSampleTrainingType
     sample_files = [
         os.path.join(sample_csv_collection, file)
@@ -352,9 +366,7 @@ def test_make_data_stream_source_list_of_csv_files(
     validate_data_stream(data_stream, 6, SampleTrainingType)
 
 
-def test_make_data_stream_source_list_of_jsonl_files(
-    sample_train_service, sample_jsonl_collection
-):
+def test_make_data_stream_source_list_of_jsonl_files(sample_jsonl_collection):
     stream_type = caikit.interfaces.common.data_model.DataStreamSourceSampleTrainingType
     sample_files = [
         os.path.join(sample_jsonl_collection, file)
@@ -374,7 +386,7 @@ def test_make_data_stream_source_list_of_jsonl_files(
 #################
 
 
-def test_make_data_stream_source_jsondir(sample_train_service, sample_json_collection):
+def test_make_data_stream_source_jsondir(sample_json_collection):
     stream_type = caikit.interfaces.common.data_model.DataStreamSourceSampleTrainingType
     ds = stream_type(directory=Directory(dirname=sample_json_collection))
     assert isinstance(ds, DataStreamSourceBase)
@@ -385,7 +397,7 @@ def test_make_data_stream_source_jsondir(sample_train_service, sample_json_colle
     validate_data_stream(data_stream, 3, SampleTrainingType)
 
 
-def test_make_data_stream_source_csvdir(sample_train_service, sample_csv_collection):
+def test_make_data_stream_source_csvdir(sample_csv_collection):
     stream_type = caikit.interfaces.common.data_model.DataStreamSourceSampleTrainingType
     ds = stream_type(
         directory=Directory(dirname=sample_csv_collection, extension="csv")
@@ -398,9 +410,7 @@ def test_make_data_stream_source_csvdir(sample_train_service, sample_csv_collect
     validate_data_stream(data_stream, 6, SampleTrainingType)
 
 
-def test_make_data_stream_source_jsonldir(
-    sample_train_service, sample_jsonl_collection
-):
+def test_make_data_stream_source_jsonldir(sample_jsonl_collection):
     stream_type = caikit.interfaces.common.data_model.DataStreamSourceSampleTrainingType
     ds = stream_type(
         directory=Directory(dirname=sample_jsonl_collection, extension="jsonl")
@@ -439,21 +449,21 @@ def test_data_stream_operators():
 # Error Tests
 
 
-def test_make_data_stream_source_invalid_file_raises(sample_train_service):
+def test_make_data_stream_source_invalid_file_raises():
     stream_type = caikit.interfaces.common.data_model.DataStreamSourceSampleTrainingType
     ds = stream_type(file=File(filename="invalid_file"))
     with pytest.raises(CaikitRuntimeException):
         ds.to_data_stream()
 
 
-def test_make_data_stream_source_invalid_dir_raises(sample_train_service):
+def test_make_data_stream_source_invalid_dir_raises():
     stream_type = caikit.interfaces.common.data_model.DataStreamSourceSampleTrainingType
     ds = stream_type(directory=Directory(dirname="invalid_dir"))
     with pytest.raises(CaikitRuntimeException):
         ds.to_data_stream()
 
 
-def test_data_stream_source_single_oneof(sample_train_service):
+def test_data_stream_source_single_oneof():
     stream_type = caikit.interfaces.common.data_model.DataStreamSourceSampleTrainingType
     with pytest.raises(CaikitRuntimeException):
         stream_type(
@@ -462,7 +472,7 @@ def test_data_stream_source_single_oneof(sample_train_service):
         )
 
 
-def test_make_data_stream_source_invalid_ext_dir(sample_train_service):
+def test_make_data_stream_source_invalid_ext_dir():
     with tempfile.TemporaryDirectory() as tempdir:
         fname = os.path.join(tempdir, f"sample.txt")
         with open(fname, "w") as handle:
@@ -476,9 +486,7 @@ def test_make_data_stream_source_invalid_ext_dir(sample_train_service):
         assert "Extension not supported!" in e.value.message
 
 
-def test_make_data_stream_source_no_files_w_ext_dir(
-    sample_train_service, sample_jsonl_collection
-):
+def test_make_data_stream_source_no_files_w_ext_dir(sample_jsonl_collection):
     stream_type = caikit.interfaces.common.data_model.DataStreamSourceSampleTrainingType
     ds = stream_type(
         directory=Directory(dirname=sample_jsonl_collection, extension="csv")
@@ -507,7 +515,7 @@ def test_make_data_stream_source_non_json_array_errors(tmp_path):
         ds.to_data_stream()
 
 
-def test_s3_not_implemented(sample_train_service):
+def test_s3_not_implemented():
     stream_type = caikit.interfaces.common.data_model.DataStreamSourceSampleTrainingType
     ds = stream_type(s3files=S3Files())
     # Explicit .to_data_stream will fail