Interactive files #1063
Interactive files #1063
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
…ndard library that has already been tested)
For now, I'm sandboxing all of these aesthetic improvements to Pixie. We can talk about more widespread adoption after we've fully fleshed them out. |
| @@ -27,6 +27,7 @@ | |||
| "outputs": [], | |||
There was a problem hiding this comment.
Line #7. fc.create_file_chooser(base_dir, dir_to_assign="cell_clustering_params_dir", file_to_assign="cell_clustering_params_path")
Is there any reason the user would have a different file path than the default /base_dir/pixie/example_pixel_output_dir/cell_clustering_params.json? I feel like this could be an automatic assignment rather than having the user find and select a nested file like this. Maybe selecting the pixel_output_dir would make more sense, e.g. if the user had multiple pixel clustering runs.
Reply via ReviewNB
There was a problem hiding this comment.
Don't think so, good idea to just leave it as an automatic assignment.
There was a problem hiding this comment.
So to load the cell_clustering_params.json file the user will need to specify the path to the pixel_output_dir created in the previous notebook. Still agreed we don't need them to explicitly specify the cell_clustering_params.json file though.
|
Just a minor comment on selecting a nested file which should have the same name for all users, so it probably can just be assigned by us rather than making them search for and choose it. |
|
Not a whole lot we can do about coverage, the original repo didn't have any test functions written for it. Don't think it's worth writing our own either. |
|
@cliu72 just had to make one small tweak to ensure the function can retrieve the file names consistently on top of the full parent paths. |
| @@ -31,16 +31,42 @@ | |||
| "from ark.utils import data_utils, example_dataset, plot_utils\n", | |||
There was a problem hiding this comment.
Small comment - can we update this to say "If you'd like to test the features in ark with an example dataset, leave use_example_dataset as True. If you're using your own data, set as False ."
Reply via ReviewNB
| @@ -31,16 +31,42 @@ | |||
| "from ark.utils import data_utils, example_dataset, plot_utils\n", | |||
There was a problem hiding this comment.
Again very small comment - I think there a small typo, should be ../data/example_dataset. Also, I don't think we need quotes around "../data/example_dataset" , since it's already displaying as code.
Reply via ReviewNB
| @@ -31,16 +31,42 @@ | |||
| "from ark.utils import data_utils, example_dataset, plot_utils\n", | |||
There was a problem hiding this comment.
To be consistent with the way this path is specified for the example dataset above, can we change{base_dir}/image_data to ../data/example_dataset/image_data ?
Reply via ReviewNB
| @@ -42,20 +42,44 @@ | |||
| "from ark.utils import data_utils, example_dataset, plot_utils\n", | |||
There was a problem hiding this comment.
Same as above - can we update this to say "If you'd like to test the features in ark with an example dataset, leave use_example_dataset as True. If you're using your own data, set as False ."
Reply via ReviewNB
| @@ -42,20 +42,44 @@ | |||
| "from ark.utils import data_utils, example_dataset, plot_utils\n", | |||
There was a problem hiding this comment.
Same as above - I think there a small typo, should be ../data/example_dataset. Also, I don't think we need quotes around "../data/example_dataset" , since it's already displaying as code.
Also, I think the last sentence of the base_dir bullet point should say "...created during cell clustering" (not pixel clustering).
Reply via ReviewNB
| @@ -42,20 +42,44 @@ | |||
| "from ark.utils import data_utils, example_dataset, plot_utils\n", | |||
There was a problem hiding this comment.
To be consistent with the way this path is specified for the example dataset above, can we change{base_dir}/pixie/example_pixel_output_dir to ../data/example_dataset/pixie/example_pixel_output_dir ?
Reply via ReviewNB
| @@ -42,20 +42,44 @@ | |||
| "from ark.utils import data_utils, example_dataset, plot_utils\n", | |||
There was a problem hiding this comment.
To be consistent with the way this path is specified for the example dataset above, can we change{base_dir}/segmentation/cell_table/cell_table_size_normalized.csv to ../data/example_dataset/segmentation/cell_table/cell_table_size_normalized.csv? And also get rid of the quotes.
Reply via ReviewNB
There was a problem hiding this comment.
Is there a way to have the paths that were selected by the user persist after saving the notebook? When the pixel clustering notebook crashes or if people break up their run over several days, it's annoying to have to re-select multiple paths every time they start the notebook.
What is the purpose of this PR?
Closes #1029. Add interactive file selection to the notebooks.
How did you implement your changes
Use
ipyfilechooserto allow users to interactively set home directories, such asbase_dir,tiff_dir, etc.Remaining issues
Start with notebooks 2 and 3. Gradually expand to include the others.