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

Joystick #66

Closed
wants to merge 9 commits into from
Closed

Conversation

ahmad11111111111
Copy link
Collaborator

-added read method and label is no longer assumed.

  • would have merged into a new branch made from an issue but i cant make branches in this repository for some reason

Copy link
Collaborator Author

@ahmad11111111111 ahmad11111111111 left a comment

Choose a reason for hiding this comment

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

looks good

@kevincar
Copy link
Member

looks good

Not sure what this means? There are still comments you'll need to review.

@@ -7,13 +7,14 @@
if TYPE_CHECKING:
from ..session import Session # type: ignore


#make label be defined after session variable
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove this comment? This seems like a personal Todo

Copy link
Member

Choose a reason for hiding this comment

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

@@ -43,7 +44,8 @@ def __init__(
preamp_filter : str
Optionally supplied preamp filter settings for saving into the data
of the edf file
init_read_fn : Union[Tuple[str, List, Dict], Callable]
init_read_fn :ead functions / joystcik device pysio instrument
eeg_instrument = EEGIns Union[Tuple[str, List, Dict], Callable]
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing this line was inserted by mistake?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

This is still not resolved

Copy link
Member

Choose a reason for hiding this comment

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

@ahmad11111111111 Was this changed?

Copy link
Collaborator Author

@ahmad11111111111 ahmad11111111111 left a comment

Choose a reason for hiding this comment

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

check now

@ahmad11111111111
Copy link
Collaborator Author

i pushed the changes

@@ -43,7 +44,8 @@ def __init__(
preamp_filter : str
Optionally supplied preamp filter settings for saving into the data
of the edf file
init_read_fn : Union[Tuple[str, List, Dict], Callable]
init_read_fn :ead functions / joystcik device pysio instrument
eeg_instrument = EEGIns Union[Tuple[str, List, Dict], Callable]
Copy link
Member

Choose a reason for hiding this comment

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

This is still not resolved

init_read_fn: Union[Tuple[str, list, Dict], Callable] = lambda: None,
read_fn: Union[Tuple[str, list, Dict], Callable] = lambda: None,
**kwargs
init_read_fn :ead functions / joystcik device pysio instrument
Copy link
Member

Choose a reason for hiding this comment

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

This also seems to be an error.

@kevincar kevincar self-requested a review February 27, 2024 23:48
Copy link
Member

@kevincar kevincar left a comment

Choose a reason for hiding this comment

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

See the final comment

@ahmad11111111111
Copy link
Collaborator Author

ok give me a minute. i think i dug myself into a hole here.

@ahmad11111111111
Copy link
Collaborator Author

i think the rencent commits may have made things worse

@ahmad11111111111
Copy link
Collaborator Author

I still have no idea what happened with the test subject file

@kevincar
Copy link
Member

I still have no idea what happened with the test subject file

No worries. I had accidentally deleted it during the review process. You can either simply copy it from the repositories history, or I can add it in after the merge

@ahmad11111111111
Copy link
Collaborator Author

i think i got it. I think my brain is still melted a bit from all of those lab conduct courses

@kevincar kevincar linked an issue Feb 28, 2024 that may be closed by this pull request
@kevincar
Copy link
Member

Awesome. Just one more comment to resolve https://github.com/umnil/libbids/pull/66/files#r1495813483

@kevincar kevincar deleted the branch umnil:develop June 17, 2024 23:19
@kevincar kevincar closed this Jun 17, 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.

universal device support for physio instrument
2 participants