indica-mcf / Indica

Integrated DiagnostiC Analysis
GNU General Public License v3.0
6 stars 0 forks source link

Add ability to call ADAS provided readers #226

Open pixelifytica opened 1 year ago

pixelifytica commented 1 year ago

ADAS members have access to pre-made reading libraries for a variety of ADF files. Suggest adding capability to drop these in (in a similar manner to the PPFReader) if available, as it would speed up adding new ADAS readers before writing our own version.

I see 2 potential options:

  1. Have a separate reader; Perhaps rename the current ADASReader to OpenADASReader and use ADASReader as ADAS library interface
  2. Have a check in ADASReader to optionally use the ADAS library if available and requested, falling back to built-in implementation

Would be helpful for #222

marcosertoli commented 1 year ago

Sounds good to me Evie.

But the underlying functions could be the same as the files are identical.

One thing that the current reader does is save also the configuration e.g. for PECs. We might want to retain that also in the regular ADAS reader if possible.

Marco


From: Evie Litherland-Smith @.> Sent: Thursday, March 9, 2023 4:01:40 PM To: ukaea/Indica @.> Cc: Subscribed @.***> Subject: [ukaea/Indica] Add ability to call ADAS provided readers (Issue #226)

ADAS members have access to pre-made reading libraries for a variety of ADF files. Suggest adding capability to drop these in (in a similar manner to the PPFReader) if available, as it would speed up adding new ADAS readers before writing our own version.

I see 2 potential options:

  1. Have a separate reader; Perhaps rename the current ADASReader to OpenADASReader and use ADASReader as ADAS library interface
  2. Have a check in ADASReader to optionally use the ADAS library if available and requested, falling back to built-in implementation

Would be helpful for #222https://github.com/ukaea/Indica/issues/222

— Reply to this email directly, view it on GitHubhttps://github.com/ukaea/Indica/issues/226, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ARZSM6HWMC7ISH5ODS2FQNTW3H5GJANCNFSM6AAAAAAVVJX44U. You are receiving this because you are subscribed to this thread.Message ID: @.***>

pixelifytica commented 1 year ago

@marcosertoli what I was thinking was keep the same class structure but replace the logic of the get_adfxx functions to just download the file and call adas.read_adfxx to get the data, aiming to keep everything else the same so the two are completely interchangeable.

Thoughts on if you prefer the split class or single "smart" class approach? I'd probably err towards split but that's just because I like things to be as explicit and transparent as possible.

Could you point me to where it saves the configuration? I can't see it, but yes I agree we want to retain as many features as possible between the two methods