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

Update from the discussion on Issue #1 #2

Merged
merged 19 commits into from
Oct 6, 2021

Conversation

yambottle
Copy link

Points that need to improve:

  1. Change package name from +neuropixels to +npxtoolkit
  2. ClassDiagram 1 - 1...0 relationship should be 1 - *, also updated UsecaseDiagram to resolve some confusion
  3. Add an Assembly class as the top level object, also changed Pipeline to Session in order to avoid concept conflicts
  4. Now the object level will be Assembly->Session->Stage->JobBase
  5. Change function assemble() to add_session() to avoid confusion
  6. Include log4m at +npxtoolkit/+internal/+logging/
  7. Will work on the 'non-fork' repo

@yambottle
Copy link
Author

@dimitri-yatsenko @iamamutt Please take a look when you guys are available, I made those changes followed by the discussion in Issue 1

launch.m Show resolved Hide resolved
@iamamutt
Copy link

iamamutt commented Oct 5, 2021

I think the class design is pretty clear. I think later down the line we may want to simplify launch.m. Maybe providing some helper functions to wrap up some of the steps. I know this this is the way it kind of is in ecephys so if we just want to try to stick to that then this may be okay.

@dimitri-yatsenko dimitri-yatsenko merged commit b754c58 into datajoint-company:main Oct 6, 2021
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.

3 participants