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.

Sign up for GitHub

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

Convert formats to yaml with ids #76

Merged
merged 14 commits into from
Dec 9, 2024
Merged

Convert formats to yaml with ids #76

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
f916757
prepare rm sd formats
sneakers-the-rat Nov 12, 2024
e9e4738
rm sdcard formats
sneakers-the-rat Nov 12, 2024
ffd5af0
narrowing in on a bug in streamdaq format...
sneakers-the-rat Nov 12, 2024
a4d0c56
broken version loaded from yaml file
sneakers-the-rat Nov 12, 2024
adee0dc
working version again
sneakers-the-rat Nov 12, 2024
c6bb772
oop forgot to actually commit the relevant config
sneakers-the-rat Nov 12, 2024
0d9d80f
now back to not working version
sneakers-the-rat Nov 12, 2024
27e39ac
correct instantiation
sneakers-the-rat Nov 14, 2024
9d698e6
implement config from id in streaming daq
sneakers-the-rat Nov 16, 2024
7db10db
instantiate streamdaq from id in cli and tests
sneakers-the-rat Nov 16, 2024
8333a5f
rm formats module
sneakers-the-rat Nov 16, 2024
e8aee8c
cleanup older impls
sneakers-the-rat Nov 16, 2024
0d6910e
Merge branch 'main' into formats-to-yaml
sneakers-the-rat Dec 9, 2024
a736c5b
instantiate click type
sneakers-the-rat Dec 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions docs/api/formats/index.md
View file
Open in desktop

This file was deleted.

7 changes: 0 additions & 7 deletions docs/api/formats/sdcard.md
View file
Open in desktop

This file was deleted.

7 changes: 0 additions & 7 deletions docs/api/formats/stream.md
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion docs/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ device/update_controller
:caption: API:

api/devices
api/formats/index
api/io
api/logging
api/models/index
Expand Down
2 changes: 0 additions & 2 deletions miniscope_io/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from importlib import metadata
from pathlib import Path

from miniscope_io.io import SDCard
from miniscope_io.logging import init_logger
from miniscope_io.models.config import Config

Expand All @@ -19,7 +18,6 @@
"DATA_DIR",
"CONFIG_DIR",
"Config",
"SDCard",
"init_logger",
]

Expand Down
31 changes: 31 additions & 0 deletions miniscope_io/cli/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
"""
Shared CLI utils
"""

from os import PathLike
from pathlib import Path
from typing import Optional

from click import Context, Parameter, ParamType


class ConfigIDOrPath(ParamType):
"""
A custom click type to accept either a config `id` or a path
as input, resolving relative paths first against
the current working directory and second against the user config directory.
"""

name = "config-id-or-path"

def convert(
self, value: str | PathLike[str], param: Optional[Parameter], ctx: Optional[Context]
) -> str | Path:
"""
If something looks like a yaml file, return as a path, otherwise return unchanged.

Don't do validation here, the Config model will handle that on instantiation.
"""
if value.endswith(".yaml") or value.endswith(".yml"):
value = Path(value)
return value
10 changes: 8 additions & 2 deletions miniscope_io/cli/stream.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import click

from miniscope_io.cli.common import ConfigIDOrPath
from miniscope_io.stream_daq import StreamDaq


Expand All @@ -24,8 +25,13 @@ def _common_options(fn: Callable) -> Callable:
"-c",
"--device_config",
required=True,
help="Path to device config YAML file for streamDaq (see models.stream.StreamDevConfig)",
type=click.Path(exists=True),
help=(
"Either a config `id` or a path to device config YAML file for streamDaq "
"(see models.stream.StreamDevConfig). If path is relative, treated as "
"relative to the current directory, and then if no matching file is found, "
"relative to the user `config_dir` (see `mio config --help`)."
),
type=ConfigIDOrPath,
)(fn)
return fn

Expand Down
41 changes: 41 additions & 0 deletions miniscope_io/data/config/wirefree/sd-layout-battery.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
id: wirefree-sd-layout-battery
Copy link
Collaborator

Choose a reason for hiding this comment

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

I might not fully understand the benefit of having a dedicated id field when we could also use the file path directly. (I assume it'll be one configuration per one YAML file here.)

  • Can't we use the stem path under config as a unique ID to call configs? Like wirefree/sd-layout-battery for this example or maybe user/some-config if it's a user directory.
  • I feel there could be some confusion with duplicate IDs, and ambiguous IDs, especially because they will be hidden from the human perspective. We could add tests to prevent duplicates, but I think using file paths as unique identifiers is more intuitive.
  • There will be multiple configs for one device. I think we'll have higher risks of ambiguous configs IDs getting pushed (like ScopeX-1, ScopeX-2, ScopeX-old, ScopeX-latest, and ScopeY for a device called ScopeX), compared to when we just use path/filename as IDs.
  • Being able to have nice versions in IDs feels nice, but I think it'll be pretty confusing if the filename doesn't sync with it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ope responded to this as a global comment before i read the individual comments. Responded to most of these there, but for continuity's sake: currently we get both, and there are reasons to have both, and they don't really trade off.

I feel there could be some confusion with duplicate IDs, and ambiguous IDs,

this would be an interface question. so we would want something like

$ mio config --show
id                    path
sd-layout-battery     miniscope_io/data/configs/sd-layout-battery.yaml
my-config             {user_dir}/configs/my-config.yaml
my-config             {user_dir}/configs/my-config-2.yaml

but again you can always specify something as a path if you want to.

There will be multiple configs for one device. I think we'll have higher risks of ambiguous configs IDs getting pushed (like ScopeX-1, ScopeX-2, ScopeX-old, ScopeX-latest, and ScopeY for a device called ScopeX), compared to when we just use path/filename as IDs.

the goal is that we don't want to have scope-v1, scope-v2, etc. The idea behind having the id is that we want to have a way of identifying unique configurations semantically, so then different versions could each be id: scope-default with a different mio_version field to identify that they are related configs that are for different versions of the scope. Then the id only contains the minimal semantic information necessary to identify it: like one would usually just use wireless to run their wireless miniscope, but if they had some custom configuration they could use wireless-mycustomconfig everywhere and not have to worry about keeping the rest of their code up to date with the current location of that file, or the different file names as the version changes.

Being able to have nice versions in IDs feels nice, but I think it'll be pretty confusing if the filename doesn't sync with it.

Filenames are unique identifiers to the system, but we want to provide an interface that potentially allows people to have multiple versions of the same configuration, and exposing them in the filesystem makes for nicer ux than just having them in a database somewhere. so that's the compromise - have an id field and allow it to be decoupled from the filename.

if someone wants to refer to something by the filename, then they just go path/filename.yaml. if someone wants to refer to something by the id, they go myconfig. one can make the filename match the if they want to, but they have the ability to make it orthogonal to filename in cases where that's useful.

There will be multiple configs for one device. I think we'll have higher risks of ambiguous configs IDs getting pushed (like ScopeX-1, ScopeX-2, ScopeX-old, ScopeX-latest, and ScopeY for a device called ScopeX), compared to when we just use path/filename as IDs

mio_model: miniscope_io.models.sdcard.SDLayout
mio_version: v5.0.0
sectors:
header: 1022
config: 1023
data: 1024
size: 512
write_key0: 226277911
write_key1: 226277911
write_key2: 226277911
write_key3: 226277911
word_size: 4
header:
gain: 4
led: 5
ewl: 6
record_length: 7
fs: 8
delay_start: 9
battery_cutoff: 10
config:
width: 0
height: 1
fs: 2
buffer_size: 3
n_buffers_recorded: 4
n_buffers_dropped: 5
buffer:
linked_list: 1
frame_num: 2
buffer_count: 3
frame_buffer_count: 4
write_buffer_count: 5
dropped_buffer_count: 6
timestamp: 7
write_timestamp: 9
length: 0
data_length: 8
battery_voltage: 10
version: 0.1.1
40 changes: 40 additions & 0 deletions miniscope_io/data/config/wirefree/sd-layout.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
id: wirefree-sd-layout
mio_model: miniscope_io.models.sdcard.SDLayout
mio_version: v5.0.0
sectors:
header: 1022
config: 1023
data: 1024
size: 512
write_key0: 226277911
write_key1: 226277911
write_key2: 226277911
write_key3: 226277911
word_size: 4
header:
gain: 4
led: 5
ewl: 6
record_length: 7
fs: 8
delay_start: 9
battery_cutoff: 10
config:
width: 0
height: 1
fs: 2
buffer_size: 3
n_buffers_recorded: 4
n_buffers_dropped: 5
buffer:
linked_list: 1
frame_num: 2
buffer_count: 3
frame_buffer_count: 4
write_buffer_count: 5
dropped_buffer_count: 6
timestamp: 7
write_timestamp: null
length: 0
data_length: 8
battery_voltage: null
14 changes: 14 additions & 0 deletions miniscope_io/data/config/wireless/stream-buffer-header.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
id: stream-buffer-header
mio_model: miniscope_io.models.stream.StreamBufferHeaderFormat
mio_version: "v5.0.0"
linked_list: 0
frame_num: 1
buffer_count: 2
frame_buffer_count: 3
write_buffer_count: 4
dropped_buffer_count: 5
timestamp: 6
write_timestamp: 8
pixel_count: 7
battery_voltage_raw: 9
input_voltage_raw: 10
4 changes: 4 additions & 0 deletions miniscope_io/data/config/WLMS_v02_200px.yml → ...o/data/config/wireless/wireless-200px.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
id: wireless-200px
mio_model: miniscope_io.models.stream.StreamDevConfig
mio_version: "v5.0.0"

# capture device. "OK" (Opal Kelly) or "UART"
device: "OK"

Expand Down
9 changes: 0 additions & 9 deletions miniscope_io/formats/__init__.py
View file
Open in desktop

This file was deleted.

80 changes: 0 additions & 80 deletions miniscope_io/formats/sdcard.py
View file
Open in desktop

This file was deleted.

20 changes: 0 additions & 20 deletions miniscope_io/formats/stream.py
View file
Open in desktop

This file was deleted.

7 changes: 5 additions & 2 deletions miniscope_io/io.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from miniscope_io.logging import init_logger
from miniscope_io.models.data import Frame
from miniscope_io.models.sdcard import SDBufferHeader, SDConfig, SDLayout
from miniscope_io.types import ConfigSource


class BufferedCSVWriter:
Expand Down Expand Up @@ -104,9 +105,11 @@ class SDCard:

"""

def __init__(self, drive: Union[str, Path], layout: SDLayout):
def __init__(
self, drive: Union[str, Path], layout: Union[SDLayout, ConfigSource] = "wirefree-sd-layout"
):
self.drive = drive
self.layout = layout
self.layout = SDLayout.from_any(layout)
self.logger = init_logger("SDCard")

# Private attributes used when the file reading context is entered
Expand Down
5 changes: 3 additions & 2 deletions miniscope_io/models/buffer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@
from typing import Type, TypeVar

from miniscope_io.models import Container, MiniscopeConfig
from miniscope_io.models.mixins import ConfigYAMLMixin


class BufferHeaderFormat(MiniscopeConfig):
class BufferHeaderFormat(MiniscopeConfig, ConfigYAMLMixin):
"""
Format model used to parse header at the beginning of every buffer.

Expand Down Expand Up @@ -86,7 +87,7 @@ def from_format(
"""

header_data = dict()
for hd, header_index in format.model_dump().items():
for hd, header_index in format.model_dump(exclude=set(format.HEADER_FIELDS)).items():
if header_index is not None:
header_data[hd] = vals[header_index]

Expand Down
4 changes: 3 additions & 1 deletion miniscope_io/models/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ class Config(BaseSettings):
description="Base directory to store configuration and other temporary files, "
"other paths are relative to this by default",
)
config_dir: Path = Field(Path("config"), description="Location to store user configs")
log_dir: Path = Field(Path("logs"), description="Location to store logs")

logs: LogConfig = Field(LogConfig(), description="Additional settings for logs")

@field_validator("base_dir", mode="before")
Expand All @@ -100,7 +102,7 @@ def folder_exists(cls, v: Path) -> Path:
@model_validator(mode="after")
def paths_relative_to_basedir(self) -> "Config":
"""If relative paths are given, make them absolute relative to ``base_dir``"""
paths = ("log_dir",)
paths = ("log_dir", "config_dir")
for path_name in paths:
path = getattr(self, path_name) # type: Path
if not path.is_absolute():
Expand Down
Loading
Loading