deprecating save_ensemble_data and read_ensemble_data

[pavlis]

As we have been discussing I am in the processing of a major revision of the Database class. One of the things I did that we discussed in our weekly meetings is move to deprecate the series of functions called save_ensemble_data, save_ensemble_data_group, read_ensemble_data, and read_ensemble_group. The new design wraps the same functionality into to simpler functions called just save_data and read_data. The next revision will still support calls to the methods with the "ensemble" name, but they will do little more than call the new save_data or read_data method that supports ensembles directly. Note the new functionality includes the "group" concept automatically where we support fast reads of data stored as "binary" by reading all the data from the ensemble stored in a single file. It actually goes further and accepts mixed input for any ensemble as formatted or binary data and will take all binary data inputs and try to read them efficiently automatically. The point is to emphasize this new api is much simpler and adds significant functionality for ensembles.

The above was mostly background for the record. The design above was finalized outside this public forum in our weekly meetings. The issue I want to raise here is a minor collision in the revised API that is breaking current tests. The old signature for save_ensemble_data is this:

def save_ensemble_data(
        self,
        ensemble_object,
        mode="promiscuous",
        storage_mode="gridfs",
        dir_list=None,
        dfile_list=None,
        exclude_keys=None,
        exclude_objects=None,
        collection=None,
        data_tag=None,
        alg_name="save_ensemble_data",
        alg_id="0",
    ):

The signature for save_data changed slightly from v1 and is the following:

 def save_data(
        self,
        mspass_object,
        return_data=False,
        return_list=["_id"],
        mode="promiscuous",
        storage_mode="gridfs",
        dir=None,
        dfile=None,
        format=None,
        overwrite=False,
        exclude_keys=None,
        collection=None,
        data_tag=None,
        save_history=False,
        alg_name="save_data",
        alg_id="0",
    ):

The issue I want to focus on here is the mismatch in concept between ensembles and atomic data that the old API supported through the argument called exclude_objects. That argument was required to be a list of integer ensemble index values that were not to be saved. e.g. if we set exclude_objects=[3, 5] then on writing mspass_object.member[3] and mspass_object.member[5] would be silently skipped in writing.

I do not remember what we were thinking when we implemented exclude_objects, but it seems both unnecessary an undesirable. If someone wanted to exclude some data it would be much clearer in the code to do so explicitly with a kill or by popping the member off the container. I urge move toward the a future where we drop this from the API. I am going to have the wrappers that handle the backward compatibility issue an additional warning if the workflow attempts to use the exclude_objects list. It is trivial to implement so the revision will handle that feature, but I think we should move to completely remove that functionality in a future release that is to be determined. Alternatively, I could just remove it now and revise the tests that are breaking. What do you think we should do?

[pavlis]

I realize I forgot a second detail in the revised API. The save_ensemble_data method also had two other parameters that were an awkward way to handle something. That is, for atomic data we have the parameters
dir and dfile that can be used for save to files. The values are just strings. For ensembles we defined dir_list and dfile_list that were required to be a list of directory and file names respectively for which the data were to be written. That was a very awkward way to do that, and was completely unnecessary.

We didn't discuss this detail of the algorithm, but my revised Database.save_data method handles this differently for ensembles. That is, it checks the value of the dir and dfile arguments. If they are None type and storage_mode="files" it will use try to fetch these attributes from object "header" (Metadata). Note for atomic data that means the file name can vary with each datum. For ensembles it means we fetch the attributes form the ensemble Metadata container. I made this bombproof using logic borrowed from the current implementation. That is, on file-based output if neither the arg values nor Metadata values are defined the default will try to write files with dir as the current directory and dfile generated by a uuid generator (a private method that exists in the current implementation called _get_dfile_uuid.

Opinions on this change?