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

Revise variable names to align with changes to element-calcium-imaging #26

Closed
CBroz1 opened this issue Apr 28, 2022 · 6 comments · Fixed by #38
Closed

Revise variable names to align with changes to element-calcium-imaging #26

CBroz1 opened this issue Apr 28, 2022 · 6 comments · Fixed by #38
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@CBroz1
Copy link
Contributor

CBroz1 commented Apr 28, 2022

See this discussion. By aligning variable names across schemas (e.g., acquisition_software -> acq_software), we allow users' understanding of one to transfer to the other via stylistic consistency.

Revert miniscope where possible to calcium-imaging convention?

@CBroz1 CBroz1 added the good first issue Good for newcomers label Apr 28, 2022
@tdincer
Copy link
Contributor

tdincer commented Apr 28, 2022

This is a big operation as it's not backward compatible. It would affect the SciOps operations.

@CBroz1
Copy link
Contributor Author

CBroz1 commented Apr 28, 2022

@kabilar - Therefore miniscope should change to match calcium imaging, or we should carry the differences?

@CBroz1 CBroz1 removed the good first issue Good for newcomers label Apr 28, 2022
@CBroz1 CBroz1 transferred this issue from datajoint/element-calcium-imaging May 4, 2022
@CBroz1 CBroz1 changed the title Revise variable names to align with changes to element-miniscope Revise variable names to align with changes to element-calcium imaging May 4, 2022
@CBroz1 CBroz1 changed the title Revise variable names to align with changes to element-calcium imaging Revise variable names to align with changes to element-calcium-imaging May 4, 2022
@CBroz1 CBroz1 added this to the NIH U24 Year 3 milestone May 4, 2022
@kabilar
Copy link
Collaborator

kabilar commented May 4, 2022

Thanks @CBroz1 @tdincer. Let's revert this change. I had made the acq_software to acquisition_software update since this field was now in the sessions CSV file. And the user would have to interpret the acq_software heading. Since this update would involve many repositories, we can revert acquisition_software to acq_software.

@kabilar kabilar removed their assignment May 4, 2022
@CBroz1 CBroz1 modified the milestones: 2022Q2, 2022Q3 May 20, 2022
@CBroz1 CBroz1 added the enhancement New feature or request label Jun 15, 2022
@CBroz1 CBroz1 self-assigned this Dec 29, 2022
@CBroz1
Copy link
Contributor Author

CBroz1 commented Dec 29, 2022

I've identified the following places where I plan to make changes/preserve differences

  • Revise table structure to reflect Element Calcium imaging
    • acquisition_software -> acq_software
    • recording_directory: remove in favor of session.SessionDirectory
    • Segmentation.Mask.mask_id -> mask to reflect calcium-imaging
    • Move MaskType to after ProcessingParamSet to match ordering
    • extraction_method varchar(200) -> varchar(32)
  • Preserving differences
    • AnatomicalLocation differs, but this is workflow-calcium-imaging bug (see #57)
    • RecordingInfo.File has a file_id that calcium-imaging does not. Paths should be secondary attributes in case they change
    • Many tables in calcium imaging permit z-depth information. Miniscope is 2D for now

@kabilar
Copy link
Collaborator

kabilar commented Dec 30, 2022

Thanks @CBroz1. I agree.

For the location issue, I believe it is this link: datajoint/element-calcium-imaging#140

@CBroz1
Copy link
Contributor Author

CBroz1 commented Dec 30, 2022

For the location issue, I believe it is this link: datajoint/element-calcium-imaging#140

Oops. Yes, that's correct. Fixed above

@CBroz1 CBroz1 linked a pull request Jan 4, 2023 that will close this issue
6 tasks
@CBroz1 CBroz1 mentioned this issue Jan 4, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants