Load range level3 Fix

[sanahabhimani]

fixed the ability for load_range to read both books and .g3 files. testing on satp3 level 2 and level 3 data using config file, alias/fields, and fields arguments on the site setup.

[mhasself]

Thanks for the fix attempt here! I think there a few problems with the daq_node / node variables that mean this needs to be revised further.

I missed the earlier PR that introduced these changes, so forgive me for a few comments that go beyond the scope of the present fixes.

Please to the docstring of load_range a brief description of the two possibilities for the directory structure; either simple 5-digit folders vs. book-style naming for a specific platform.

Revise the documentation of the "daq_node"; it currently has a weird second sentence.

The "daq_node" argument will not be processed properly, if daq_node != None. It looks like daq_node and node are sharing a purpose here, and that needs to be resolved more carefully. I think you can do everything you want with just daq_node (as a function parameter that defaults to None), which gets upgraded based on the data_dir if need be.

On the variable "node" -- the code tries to guess a good value for node by looking at the data_dir. If that fails, node remains undefined, which is dangerous. Instead, you should set node = None, then search for a better value. But basically I think there's just a degeneracy with daq_node as described above.

In the folder loop, there's a test for if node in data_dir. Because of the reasons above, this will either fail (node not defined) or definitely be true (because node was set based on testing a limited set of strings against data_dir.

Have you considered simply testing, for each iteration of folder, whether any of the following patterned folders exist?:

• {folder} (e.g. 17890)
• {folder} (e.g. hk_17890_satp1, or whatever_17890_blarg, etc)

This would make the code less rigid -- we wouldn't have to hard-code the list of valid platforms into the function.

[sanahabhimani]

the docstring could be cleaner for folder_patterns arg but ready for re-review overall.

[mhasself]

Thanks. Instead of constructing a list of lists, build a single list, using extend. See comments.

[mhasself]

This looks good to me.

Please get @kmharrington approval before merge, and confirm for us that you've tested this on real level 2 and level 3 structures.

Thanks!

[sanahabhimani]

Yes, can confirm that I have tested on level2 and level3. I used satp3 data. Mostly using config file arg, with and without folder_patterns arg using .g3 files and HK books. Also made a new directory in my home dir that had a mix of HK satp3 books and satp3 .g3 files (though didn't add HK books from other telescopes..) and tested with that.

[kmharrington]

Looking like this will work out! Thanks!