Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enh/neuropixels rig #17

Closed
wants to merge 11 commits into from

Conversation

mochic
Copy link
Contributor

@mochic mochic commented Nov 18, 2023

Adds neuropixels rig mapping for mpe sync config sync.yml and mvr config mvr.ini. An additional non-mpe config mvr.mapping.json is used to map mvr camera names to camera assembly names in the rig. Aside from sync and camera assembly information, the rest of the rig information is static and rehydrated from rig.partial.json.

class NeuropixelsRigException(Exception):
"""General error for MVR."""


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go ahead and make pydantic models for these.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e.g.

class MvrCamera(BaseModel):
    assembly_name: str
    serial_number: str
    height: int
    width: int
    frame_rate: float


return rig.Rig.parse_obj({
**partial,
"modification_date": datetime.date.today(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add modification_date to the arg list. you can mention in the defaults that None means today().

"""
super().__init__(input_source, output_directory)

def _load_resource(self, path: pathlib.Path) -> str:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need this function - read_text should say something to this effect if the file does not exist.


def __init__(
self,
input_source: pathlib.Path,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

source is always a directory, right? if so you can call this input_directory

"aind-metadata-service[client]",
"scanimage-tiff-reader==1.4.1.4"
"scanimage-tiff-reader==1.4.1.4",
"pyyaml==6.0.1"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this need to be pinned?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe? Pyyaml released breaking changes between 5.0 and 6.0 so I figured pinning was safest in case of pyyaml 7.0.

@dyf
Copy link
Member

dyf commented Nov 21, 2023

@mochic let's please also make the name of the module more specific. maybe: neuropixels_mvr.

@mochic
Copy link
Contributor Author

mochic commented Apr 3, 2024

@dyf This has beeb moved to #28

@mochic mochic closed this Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants