OSOceanAcoustics / echopype

Enabling interoperability and scalability in ocean sonar data analysis
https://echopype.readthedocs.io/
Apache License 2.0
99 stars 73 forks source link

Revamp functions to check reversed timestamp in `combine_echodata` #1085

Open leewujung opened 1 year ago

leewujung commented 1 year ago

Currently combine_echodata does not perform checking of potentially reversed timestamps (this is a known issue seen on EK60/EK80 files). For pure combination of EchoData objects/files this is fine, but any subsequent operation requires using the ping_time dimension will fail if the data contains reversed time time.

In #1065 the functions to check and correct for timestamp reversal were revamped, but the mechanisms to perform this correction and keep the provenance of the changes as part of combine_echodata have not been implemented. This issue is to keep track of this needs.

@emiliom recommends adding correct_reversed_time=False, store_time_reversal_provenance=False options to combine_echodata (the last part of https://github.com/OSOceanAcoustics/echopype/pull/1065#issuecomment-1644631154), which seems a great way to allow user to decide what they want to do when combining data.

emiliom commented 1 year ago

I've found a set of 3 EK60 test files that feature reversed time (tracked them from #830). They used to be part of a test for reversed time that apparently is no longer present:

https://github.com/b-reyes/echopype/blob/3546908bde9133f49a8a3256101f1eef817804b2/echopype/tests/echodata/test_echodata_combine.py#L27-L33

def ek60_reversed_ping_time_test_data(test_path):
    files = [
        ("ncei-wcsd", "Summer2017-D20170719-T203615.raw"),
        ("ncei-wcsd", "Summer2017-D20170719-T205415.raw"),
        ("ncei-wcsd", "Summer2017-D20170719-T211347.raw"),
    ]
    return [test_path["EK60"].joinpath(*f) for f in files]