Repository navigation
Harp reader is not using the official harp package instead of harp-data - #110
Open
bruno-f-cruz wants to merge 2 commits into
Open
bruno-f-cruz wants to merge 2 commits into
bruno-f-cruz wants to merge 2 commits into
Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces
harp-pythonwithharp-data(theharp.datanamespace). Harp data is now read through the package's public API (harp.device.schema.create_device_module+harp.data.open_dataset) instead ofharp-pythonprivate internals (harp.reader._ReaderParams,harp.reader._create_register_handler).contract/harp.py: rewritten on top ofharp.data. The only custom logic left is locating thedevice.yml(WhoAmI registry, URL, register 0), which the new package does not provide, and schema metadata the device module does not carry (register descriptions,firmwareVersion).device.ymlcaching: a schema fetched by WhoAmI or URL is now downloaded once per process instead of once per device load. Failed fetches are not cached.qc/harp/*): updated to the new DataFrame layout. Bit-mask checks use bitwise ops on the integervaluecolumn, with flags taken from the package (harp.device.core.ClockConfigurationFlags, the generated module'sLickChannels).TypeError/AttributeError) when a version register has noReadmessage or the schema has nofirmwareVersion. They fail the test instead.DataStreamCollectionBase.__getitem__now returns the collection's item type, sodevice["X"]is typed asHarpRegister. Runtime behavior is unchanged.harp-python→harp-data>=0.5.0.aind_behavior_services>=0.13.9is the first release that no longer depends onharp-python, soharp-pythonis no longer installed.tests/test_contract/test_harp.pywrites real Harp.binfiles and loads them throughHarpDevice. Previously all harp tests used mocks, so a change in the data format would not have been caught.Breaking changes for downstream code
1. DataFrame layout of every register
harp-python)harp-data)MessageType, last columnmessage_type, first column"READ","WRITE","EVENT""Read","Write","Event"(pd.Categorical)df["WhoAmI"],df.RawVoltage)df["value"]payloadSpecTemperature,Encoder)temperature,encoder,torque_load_current)df["Channel0"])valuecolumnstrvaluespd.Categoricalvalues (string comparisons still work)TimeTime(unchanged)Warning
Positional access fails silently. Because
message_typemoved from last to first, code likedf.iloc[-1].values[0]orrow.iloc[0]now returns"Read"/"Event"instead of the register value. It raises no error and returns the wrong value. Always select by column name (df["value"].iloc[-1]).2. API changes in
contraqctor.contract.harpHarpDevice.device_reader(harp.reader.DeviceReader)HarpDevice.dataset_reader(harp.data.DatasetReader) andHarpDevice.device_schema(parseddevice.yml)device.device_reader.device.whoAmI/.firmwareVersiondevice.device_schema.whoAmI/.firmwareVersiondevice.device_reader.registers[...]device.dataset_reader.device_module.REGISTER_MAP(by address), or register classes by name ondevice.dataset_reader.device_moduleHarpRegisterParams(alias ofharp.reader._ReaderParams, fieldsbase_path,epoch,keep_type)keep_type,decode_enums,demux_bit_masks. The time reference (epoch) is set once per device onHarpDeviceParams, not per registerHarpRegister.from_register_reader(name, reg_reader, params)HarpRegister.from_dataset_reader(name, dataset_reader, register_class, *, description=None, params=...)HarpRegister.register_class: theharp.protocolregister class (.address,.payload_type, ...)HarpDeviceParams(show_private=...)TypeError). Private registers are never exposedHarpDeviceParams(decode_enums=True, demux_bit_masks=False)import harp,harp.read,harp.read_schema,harp.reader.*harp.data.read,harp.data.open_dataset,harp.device.schema.parse_device_schema/create_device_module3. Behavior changes
harp.data.DatasetReadercompares the schema'swhoAmIagainst thedevice.ymlin the data folder. A mismatch (e.g. aDeviceYmlByWhoAmIhint pointing at the wrong device) now raisesValueError: WhoAmI mismatchwhen the device loads. Previously this was only reported by the QC check.interfaceTypeare decoded as their native numeric type (require_converters=False), matching whatharp-pythoneffectively did.FileNotFoundError, as before. Plainharp.datawould return an empty DataFrame instead; contraqctor keeps the error sohas_dataand the read-dump QC keep their meaning.device.ymlcache: a schema fetched from a repository'smainbranch is not re-fetched until the Python process restarts.HarpHubTestSuite(clock generator checks) andHarpLicketySplitTestSuiteread the integervaluecolumn. Loading those devices withdemux_bit_masks=Truemakes those checks raiseKeyError: 'value'.Fix reference
Name conversion rules come from
harp-device, and its runtime modules match the static generated device packages: payload fields → snake_case (DutyCycle→duty_cycle), enum/flag members → SCREAMING_CASE (DIPort0→DI_PORT0). Register, enum and payload class names are kept verbatim from the YAML.Known issues (pre-existing, not addressed here)
HarpLicketySplitTestSuite.test_minimum_lick_rateandtest_refractory_period_violationscount lick offsets as licks:.where(lambda x: x == 1).indexkeeps every row, so the lick count and rate come out roughly doubled.test_lick_durationcounts correctly (lick[lick == 1].index).HarpDeviceTestSuite.test_core_versioncallsself.fail_test("Core version is not a valid semver version."), which passes the message as the result value, so the failure has no message.Testing
pytest: 183 passed, including the new integration tests intests/test_contract/test_harp.py.device.ymlof the Lickety Split (WhoAmI 1400) and Environment Sensor (1405), with synthetic frames written to disk and the QC suites run on them.examples/qc.pyon a real dataset is recommended before release.