SuperDARN / pyDARNio

Python Library for read in SuperDARN data
GNU Lesser General Public License v3.0
8 stars 2 forks source link

added new fitacf 3 fields #39

Closed mts299 closed 2 years ago

mts299 commented 2 years ago

Scope

@ecbland pointed out new fields will be added to the fitacf 3 file format thus here is the corresponding pyDARNio change for reading the fitacf files

issue: #38

Approval

Number of approvals: 1-2

Test

should be the normal read_fitacf() command we use with the a test file from #38 provided by @ecbland

ecbland commented 2 years ago

@mts299 There's a test fitacf file on the issue I opened yesterday.

mts299 commented 2 years ago

My bad :p trying to avoid staring at a screen ;)

ecbland commented 2 years ago

@mts299 In commit https://github.com/SuperDARN/pyDARNio/pull/39/commits/81e1edce9da223ad5f59e13f73a6d27d71794a57 I've separated the field names into 3 separate groups:

I tested this with fitacf2 and fitacf3 files created using the current file format, and also with the new format (fitacf3 only). Here are the 3 test files: test_files.zip

Is this sensible?

carleyjmartin commented 2 years ago

@RemingtonRohel just FYI: the DVWG has their eye on this PR, we're hoping to merge to develop after testing soon and do a new release before RST is released to have this available before the new fitacf3 are able to be made in RST. Is the borealis restructure PR something you're interested in getting into a new release?

Depending on how long it takes for the other two fitacf changes going to happen to make its way through the dev and rubber stamping phases of RST, we will try to get them in the same release too.

RemingtonRohel commented 2 years ago

@carleyjmartin Sorry for taking so long to get to this! I'm taking another look at the restructure PR today, I would love to roll these both into a new release if we can! Since there isn't much else going on with this package I think bundling them is a good idea, so I'll try to be speedy.

Shirling-VT commented 2 years ago

@ecbland I tested using the codes Carley provided above, I can print and see (x_qflg, x_p_l,...,elv_low, elv_high,...) fields in both the fitacf2 and fitacf3 files without modified format. When I print data in the fitacf3 with modified format, I can only see elv_fitted and elv_error fields but not the elv_low and elv_high fields. Are these expected output? If so, I think we can merge this PR. But I may misunderstand how you separate fields in the three files, my output does not seem to fully agree with your description below: I've separated the field names into 3 separate groups: xcf_fields: these fields are common to all fitacf files containing xcf data xcf_fields_fitacf3: the fields that are unique to fitacf3 files (elv_fitted, elv_error) xcf_fields_fitacf2: the fields that are unique to fitacf2 files (x_qflg, x_p_l,...,elv_low, elv_high,...)

What do you think?

ecbland commented 2 years ago

@Shirling-VT Thanks for testing. It sounds like you're getting the expected behaviour. The "separating field names into 3 groups" (https://github.com/SuperDARN/pyDARNio/commit/81e1edce9da223ad5f59e13f73a6d27d71794a57) is purely to prevent the "missing field" errors. If you were able to read all of the sample files then this is ready to merge.