Skip to content

Conversation

@ChucklesOnGitHub
Copy link
Member

Addresses #121

@cjsha if you could especially check how I handled the scale and offest parameters. Also, the video didn't play on my local build, I'd appreciate help with that.
@bparks13 this is about using what you programmed, I hope it explains it as intended

@ChucklesOnGitHub ChucklesOnGitHub linked an issue Feb 6, 2025 that may be closed by this pull request
@ChucklesOnGitHub ChucklesOnGitHub marked this pull request as ready for review February 6, 2025 17:11
Copy link
Member

@bparks13 bparks13 left a comment

Choose a reason for hiding this comment

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

This is a great start! I think it will be very useful for describing how to use the ephys socket. That being said, I do think there are some changes that need to be made before we can publish it.

I did go through and fix the merge conflicts, as well as fixed the broken xrefs and files. For external references, such as those for the OE GUI docs, you don't need to use xref: before the link.

I also added a hard wrap to the file at 100 characters so that it is easier to visualize and review. I am using the Rewrap extension in VS Code to make this easier to track.

@cjsha
Copy link
Member

cjsha commented Feb 6, 2025

yeah you have to be careful with rewrap bc it kills notes, tips, etc. and maybe workflow containers

@cjsha
Copy link
Member

cjsha commented Feb 7, 2025

feedback:

  • I agree with bparks that the signal chain screenshots can be of the signal chain viewport instead, but only for the first set. The rest make sense to keep as is imo.
  • super high density of blue text might be overwhelming
    image
  • the screenshots of the entire GUI UI might be too small. it might be hard to see the play button.
  • We should put a download for the signal chain if possible.
  • I ran the workflow & signal chain, works great. great work, chucklesongithub 🥇

in addition to this feedback, I made a commit in issue-121-socket_cs-edits for you to compare and see if you like. Check the commit message e94ec66 for a description of the changes I made.

@cjsha cjsha requested a review from jonnew February 11, 2025 16:04
- Tried to streamline and clarify text
- Please replace the first image with something that summarizes the end result of the tutorial
Copy link
Member

@jonnew jonnew left a comment

Choose a reason for hiding this comment

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

This is great. I committed some changes. Some comments:

  • Image of signal chain on opening page needs to be explained and given more context. TBH I didn't know what it was at first because it does not have much of the characteristic OE GUI design elements to help situate it. Its also not really described via caption or sounding text. At this point i would show a picture of the end result: the whole bonsai editor running and the whole OE GUI with data streaming probe visualizer to give the user an idea of the target.
    image
  • Avoid floating sentences. The limited customization of the docfx site makes these littel sentences appear to be floating around in the background. If you have a one line statement to make, it should probably go in in admonition
    image
  • The TCPServer nodes are named SpikeSocket and LFPSocket. Ive changed these to to SpikeServer and LFPServer as that is more descriptive.

Additionally:
- Add some images
- Fix typos
@cjsha cjsha requested a review from jonnew February 21, 2025 04:38
@jonnew
Copy link
Member

jonnew commented Feb 24, 2025

OK, this is probably good enough to merge. A major issue for all of the images in these docs is the following:

One major issue we are having with the docs is simply getting pictures of things operating a way that indicates there functionality when used for a real experiment. Its not reassuring to look at example images of data streaming but the data looks like noise or simply a malfunctioning acquisition system. For example we are writing a tutorial on how to stream data to the GUI from bonsai and this is the example of "success":

image

but it looks like a malfunctioning system. We know its due to a floating probe, maybe that somebody was tapping, but its not reassuring to the user.

Copy link
Member

@jonnew jonnew left a comment

Choose a reason for hiding this comment

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

Although my previous comments were addressed in theory, these images are not reassuring:

image

image

Because the data they show does not look like best-case or even normal scenario for a functional workflow in an experiment. We need to find a way to create images that illustrate what the user should be aiming for.

Per jon's feedback:
- Remove random colon and put period.
- Remove link to data-elements page and adjust those sentences
  accordingly
To merge-in the submodule pointer which fixes broken refs
@cjsha cjsha requested a review from jonnew February 26, 2025 17:19
@jonnew jonnew merged commit c11dd2f into main Feb 26, 2025
3 checks passed
@jonnew jonnew deleted the issue-121-socket branch February 26, 2025 18:13
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.

Tutorial: using the Ephys Socket for visualization

4 participants