schnitzer-lab / EXTRACT-public

EXTRACT is a tractable and robust automated cell extraction tool for calcium imaging, which extracts the activities of cells as time series from both one-photon and two-photon calcium imaging movies.
MIT License
62 stars 16 forks source link

July 2024 polish - this is a draft request for study #51

Closed stevevanhooser closed 2 months ago

stevevanhooser commented 2 months ago

This is a draft pull request for discussion. @fatihdinc and Vijay, let's discuss before merging this.

I believe Vijay is going to write an email summarizing the suggestions. The new tutorials are very nice! The suggestions were small and editorial in nature.

Best Steve

fatihdinc commented 2 months ago

I am going to take a quick look soon, but I realized two things:

1) The view links on the readme seem to be broken? 2) Do we want to add the links to movies in the main readme as well? Or perhaps in a readme file inside the lecture tutorial folder? Some people will likely not use the matlab online and may want to download them directly. Do we have a link for them or do we use the google drive?

Thanks!

stevevanhooser commented 2 months ago

Hi @fatihdinc -

  1. Yes, the view links cannot be placed until a) we merge the pull request to the main branch, b) wait for File Exchange to notice and update the File Exchange entry, and c) edit the README to point to the new generated view links. This is an annoying workaround for the File Exchange site but it is what we have to do these days.
  2. I edited the tutorials so that it automatically downloads the jones.h5 movie. I notice there are some other files in the Advanced tutorials that could be added. In other toolboxes we have just had the demos automatically download the files so the user doesn't have to!

Best Steve

fatihdinc commented 2 months ago

Sounds good, are we ready to merge or will you be making additional changes?

stevevanhooser commented 2 months ago

Yes, I'm ready! We will have to update it one more time to get the View links to work (but we have to do this after the first merge). Thanks! Steve

fatihdinc commented 2 months ago

Done! Thanks!