Add Intan loader

[tdincer]

No description provided.

[JaerongA]

Thanks @tdincer. Just one comment.

[kabilar]

Converted to draft to accommodate different Intan file format.

[tdincer]

rhs_perchannel_loader.py is the one developed to read per channel data. The rest of the files are taken from https://github.com/datajoint/load-rhs-notebook-python. Note that there is a COPYING file, which is the original license from the same repo.

[JaerongA]

Suggested change

	print("")
	print(
	"Reading Intan Technologies RHS2000 Data File, Version {}.{}".format(
	version["major"], version["minor"]
	)
	)
	print("")

Can we remove all these print statements?

[CBroz1]

I'd suggest migrating all element print statements to datajoint-python-formatted logging

import logging
logger = logging.getLogger('datajoint')
logger.info('existing statement')

[CBroz1]

I find this hard to read, and I would expect it would be tough for newcomers to audit/maintain as a result. I'd suggest adopting the kinds of formatting adjustments I proposed in #74

[CBroz1]

I think the best reason not to make such edits would be if we anticipated further developments from the original intan team. If the original repo is stable and unlikely to update, I would prioritize readability over exactness in the copy

[CBroz1]

Could this be a for loop?

[CBroz1]

This looks to me like it could be a dict

signal_type_lookup = dict(
  amplifier_channels = ["Voltage", "amplifier_data"]
  dc_amplifier_channels = ...
)
ylabel, signal_data_name = signal_type_lookup.get(signal_type, [None, None])
if not ylabel:
  raise ValueError("Err message")

[CBroz1]

By reserving -1 as your not-found value, you could have this function only return the this_channel value and not need to juggle two returns

[tdincer]

It's not my code.

[tdincer]

Please do not spend time on correcting the rhsutilities.py. It's not our code.

If Intan updates the code, any changes made in this file will make the comparison of this file and the original file harder.

[JaerongA]

added this to output the header

[kabilar]

It might get confusing to mix licenses within a single repo. Let's brainstorm on alternative options.

[tdincer]

I'm also concerned about that. Alternatively, we can make the entire module a separate package. But then we would need to add some lines to the COPYING. In that case, we can fork from the intan's original repo.

[kabilar]

I like the idea of forking Intan's repo and making modifications there. That would allow us to pull from the base repository as it is updated, and to contribute back to the base repository if we have fixes.

[tdincer]

I can move the new code in this PR to https://github.com/datajoint/load-rhs-notebook-python. We will need to change the repos' name, though.

[kabilar]

Perfect. How about the following:

1. Keep the rhs_perchannel_loader module in the element-interface repo, but remove the rhsutilities module.

2. Make the DataJoint fork of the load-rhs-notebook-python repo pip installable. Then we can just import the rhsutilities module in element-interface. We will still have the same issue that we have with the scanreader package, as it is not on PyPI, but let's deal with that at a later time.

3. Rename the DataJoint fork of load-rhs-notebook-python.

[tdincer]

Sure. Let's try to finish it today.

[tdincer]

@kabilar I applied all three changes you suggested.

[kabilar]

@tdincer Which function will we use to read from the single *.rhs file which contains the metadata and raw data from all channels?

[tdincer]

@tdincer Which function will we use to read from the single *.rhs file which contains the metadata and raw data from all channels?

We will use the one in the intanrhsreader as follows:

from intanrhsreader import load_file

rhsdata = load_file('filename.rhs')

[dimitri-yatsenko]

fudge factors. Please turn into named constants.