Feature Idea for Database read_data

[pavlis]

As noted a number of places I've been working on a revision to the Database class. In working through the tests I was reminded of an awkward feature of the read_data method of Database. That is, in any context except reading from the default collection (wf_TimeSeries) that argument is not really optional.

My idea is pretty simple. I suggest the write_data method and also index_miniseed should write some attribute that defines the data type a document can be used to construct. Because we use "wf" collections to define atomic data (i.e. each valid wf document defines one and only one atomic data object - a TimeSeries or Seismogram object). I then suggest each wf document should contain a data_type key-value pair. I suggest it could be either the strings "TimeSeries" or "Seismogram". We might also want to not require the mixed case and match on something like a conversion to upper or lower case, but that is a detail. Might also want to allow the result of str(type(d)) where d is an atomic datum. As you likely know that always yields a more verbose string.

Implementing this would not be hard in the new implementation. All calls to save_data and index_mseed would insert a value for the new data_type attribute that is defined by an isinstance check. That is really trivial to implement, but it will break some of the existing tests. The real need is for read_data and it presents more of a design challenge on the behavior it should have. I would suggest that if undefined we revert back to the current behavior and depend on the collection argument. If it is defined there are two situations depending on what the schema says the collection should define. That is, if the value of the new data_type attribute is consistent with the collection aok. If they are different, log a message and decide which is definitive. I presume the collection argument would override any single datum value, but I could see an argument for the reverse too.

Anyway, I wanted to get this idea down for discussion.

[pavlis]

I would like to add one more detail on this proposed change. We already do some filtering of wf document key-value pairs in save/read operations that are not optional. That is, aliases and values set as all space characters are dropped altered and dropped respectively in a save. Hence, adding data_type as a required attribute that would be forced on each save would be just a variant of that same filtering.

read_data currently has a behavior that would change slightly with this proposed change. When mode is "promiscuous" no filtering of document content is done at all except for the key-value pairs defined by the "exclude_keys" argument to the read_data method. "cautious" and "pedantic" will heavily filter the output excluding anything not defined by the schema. I think we should retain that behavior but add a nonnegotiable stripping of the redundant data_type when reading. That attribute would then only live in database documents for the cross-checking of type proposed here.

I would also note that this section of read_data would be much cleaner with this solution. Here is the existing code at the top of the read_data method that I retained so far in my revision:

        try:
            wf_collection = self.database_schema.default_name(collection)
        except MsPASSError as err:
            raise MsPASSError(
                "collection {} is not defined in database schema".format(collection),
                "Invalid",
            ) from err
        object_type = self.database_schema[wf_collection].data_type()

        if object_type not in [TimeSeries, Seismogram]:
            raise MsPASSError(
                "only TimeSeries and Seismogram are supported, but {} is requested. Please check the data_type of {} collection.".format(
                    object_type, wf_collection
                ),
                "Fatal",
            )

I realized on further thought that the value of the data_type in each document should be definitive over the type the schema wants to see using the above code segment. What I think we should do is always check type the schema expects and log a complaint if they don't match. As noted in the initial posting we should also log a complaint if data_type is undefined and we have to use the collection name to establish the object type. Normal (no log entry) would be the norm when the two types match. Actually, a point of discussion is if such an error should be one of two more severe alternatives: (1) kill the datum, or (2) abort the workflow by throwing an exception. Maybe the correct behavior is to abort by default, but have an option to only kill such data and continue. That may be the right behavior as currently you can get errors when developing a workflow that are mysterious if you don't use the right collection name for the type of data you are using. With this feature one would rarely need to specify the collection name at all on a read so a more assertive abort may be the right behavior.