OSOceanAcoustics / echopype

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

Compliance of frequency variable encoding in converted files #307

Closed leewujung closed 1 year ago

leewujung commented 3 years ago

Original

It appears that frequency has been encoded in int32 for AZFP converted files, which results into errors when computing for environmental variables #304 . Let's fix this by encoding it as float as specified in the convention.

Updated

Per @emiliom 's comments below, let's expand the scope of this issue beyond AZFP, so that we do not forget about this other side of compliance issue in conjunction with #210 .

leewujung commented 3 years ago

Closed by #309 .

emiliom commented 3 years ago

The same change will have to be made for the other sensor types (EK60, etc), right? How about expanding the scope of this issue (and reopening it) beyond AZFP, so the others are not forgotten?

leewujung commented 3 years ago

Yes I was gonna add a comment in the convention issue #210 . We can reopen this issue and do everything here. :)

leewujung commented 2 years ago

I think we can probably make this in a minor release v0.6.X once we have the main (breaking) things down, and I wonder if this is better addressed through the echopydantic route.

emiliom commented 1 year ago

The change to float was done for AZFP in PR #309 (originally to the old frequency variable).

For EK60 and EK80, frequency_nominal is already set to float, too.

I think we can close this issue.

leewujung commented 1 year ago

Sound good. I imagine this will be covered by #1043

emiliom commented 1 year ago

Thanks. Yeah, the goal in #1043 is to also check for data types.

I'll close this issue now.