Updated curation to allow user defined cube size

[MysticElephant]

Before submitting a pull request (PR), please read the contributing guide.

Description

Updated the napari plugin of cellfinder to allow user to redefine cube size and save it with the new cube sizes. The second commit should've been named "Fixed formatting issues".

What is this PR

This is an addition of a feature.

Why is this PR needed?

Currently the program only allows a specific cube size to be used for training and classification and this is the first step to allow user defined cube sizes with cellfinder. I will work on cellfinder-core and when ready, will submit a pull request in that repository.

References

#211 from cellfinder repository

How has this PR been tested?

I tested to make sure napari accepts the new value for cube size and that the saved files are saved with the correct shape. In addition, based on the cellfinder documentation for cellfinder, the cube size can only be even values and will show in the conda terminal and napari if an odd value is input and reset back to the default values.

Does this PR require an update to the documentation?

No documentation updates are needed as of yet. When full feature is released, documentation will need to be updated.

Checklist:

• The code has been tested locally
• Tests have been added to cover all new functionality (unit & integration)
• The documentation has been updated to reflect any changes
• The code has been formatted with pre-commit

[deprecated-napari-hub-preview-bot]

Preview page for your plugin is ready here:
https://preview.napari-hub.org/brainglobe/cellfinder-napari/133
Updated: 2022-09-02T09:48:27.039690

[codecov]

Codecov Report

Base: 95.42% // Head: 95.80% // Increases project coverage by +0.37% 🎉

Coverage data is based on head (4044c10) compared to base (e2d73a3).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     brainglobe/cellfinder#133      +/-   ##
==========================================
+ Coverage   95.42%   95.80%   +0.37%     
==========================================
  Files          17       17              
  Lines         808      881      +73     
==========================================
+ Hits          771      844      +73     
  Misses         37       37              

Impacted Files	Coverage Δ
cellfinder_napari/curation.py	90.69% <100.00%> (+0.86%)	⬆️
cellfinder_napari/tests/test_curation.py	100.00% <100.00%> (ø)
cellfinder_napari/tests/test_utils.py	100.00% <100.00%> (ø)
cellfinder_napari/utils.py	95.18% <100.00%> (+1.63%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[adamltyson]

Is there a reason the depth can't go above 20?

[MysticElephant]

I'll go ahead and just set a minimum value

[adamltyson]

Same comment as above. Theoretically, someone could have v. high resolution data (or huge objects to detect) and might need bigger cuboids.

[adamltyson]

Thanks for raising this PR @MysticElephant!

Currently the program only allows a specific cube size to be used for training and classification and this is the first step to allow user defined cube sizes with cellfinder. I will work on cellfinder-core and when ready, will submit a pull request in that repository.

Considering this PR relies on changes to cellfinder-core, and will also need further changes to the napari detection widget, I'm going to convert this PR to draft for now (just for my own admin purposes).

I tested to make sure napari accepts the new value for cube size and that the saved files are saved with the correct shape. In addition, based on the cellfinder documentation for cellfinder, the cube size can only be even values and will show in the conda terminal and napari if an odd value is input and reset back to the default values.

Thanks for checking this. However before merging, automated tests for these new features will need to be added. This can just be to check that the GUI is functioning properly (the actual cube extraction can be tested in cellfinder-core). Let me know if you need a hand with this.

[MysticElephant]

Thanks for raising this PR @MysticElephant!

Currently the program only allows a specific cube size to be used for training and classification and this is the first step to allow user defined cube sizes with cellfinder. I will work on cellfinder-core and when ready, will submit a pull request in that repository.

Considering this PR relies on changes to cellfinder-core, and will also need further changes to the napari detection widget, I'm going to convert this PR to draft for now (just for my own admin purposes).

I tested to make sure napari accepts the new value for cube size and that the saved files are saved with the correct shape. In addition, based on the cellfinder documentation for cellfinder, the cube size can only be even values and will show in the conda terminal and napari if an odd value is input and reset back to the default values.

Thanks for checking this. However before merging, automated tests for these new features will need to be added. This can just be to check that the GUI is functioning properly (the actual cube extraction can be tested in cellfinder-core). Let me know if you need a hand with this.

How do I add automatic automatic testing to my project? I saw something with pytest and I thought I ran that before submitting this pull request. Am I missing something?

[adamltyson]

How do I add automatic automatic testing to my project? I saw something with pytest and I thought I ran that before submitting this pull request. Am I missing something?

Yes so we run tests with pytest. These tests can be found in cellfinder-napari/cellfinder-napari/tests. This basically a collection of scripts that run some part of the code and check that what should happen, happens. Running pytest will check that the new code doesn't cause any existing tests to fail, but it doesn't ensure that the new code works, and more importantly doesn't allow others to quickly check that their changes don't mess up your contributions. To test the new code additions, typically you would either add a new test file (or add to another) with additional functions (and then run pytest again).

Not sure if this make sense? Testing is quite a large topic in itself.

[MysticElephant]

How do I add automatic automatic testing to my project? I saw something with pytest and I thought I ran that before submitting this pull request. Am I missing something?

Yes so we run tests with pytest. These tests can be found in cellfinder-napari/cellfinder-napari/tests. This basically a collection of scripts that run some part of the code and check that what should happen, happens. Running pytest will check that the new code doesn't cause any existing tests to fail, but it doesn't ensure that the new code works, and more importantly doesn't allow others to quickly check that their changes don't mess up your contributions. To test the new code additions, typically you would either add a new test file (or add to another) with additional functions (and then run pytest again).

Not sure if this make sense? Testing is quite a large topic in itself.

Hello.

I understand the whole testing process now and have added the changes from utils.py to testing. Right now, I am struggling to write the code for testing the changes in curation and was wondering if you would be able to guide me.

[adamltyson]

I understand the whole testing process now and have added the changes from utils.py to testing. Right now, I am struggling to write the code for testing the changes in curation and was wondering if you would be able to guide me.

Could you push the changes you've already made so I can take a look? I'm a bit busy at the moment, the best way to get a sense of how to test these things is probably to take a look at the other tests in cellfinder-napari, and then maybe other napari plugins.

[MysticElephant]

I understand the whole testing process now and have added the changes from utils.py to testing. Right now, I am struggling to write the code for testing the changes in curation and was wondering if you would be able to guide me.

Could you push the changes you've already made so I can take a look? I'm a bit busy at the moment, the best way to get a sense of how to test these things is probably to take a look at the other tests in cellfinder-napari, and then maybe other napari plugins.

I just pushed changes for test_utils.py and also increased the range to 1,000,000 for cube size. I was trying to see if there was a way to set positive infinity as the max value but I couldn't seem to get it to work. Would 1,000,000 be sufficient for max value?

Also, what would be the best way to implement using the shape for building the new model? I was thinking of adding 3 new arguments flags to train_yml.py so the user could specify but if there is another way that would be better, please let me know.

[adamltyson]

I just pushed changes for test_utils.py and also increased the range to 1,000,000 for cube size. I was trying to see if there was a way to set positive infinity as the max value but I couldn't seem to get it to work. Would 1,000,000 be sufficient for max value?

Yep that should be fine. Thanks.

Also, what would be the best way to implement using the shape for building the new model? I was thinking of adding 3 new arguments flags to train_yml.py so the user could specify but if there is another way that would be better, please let me know.

That sounds about right. I guess the shape could be inferred by reading in a single cuboid, but I'm not sure if it's better to pass in the shape explicitly.

[MysticElephant]

That sounds about right. I guess the shape could be inferred by reading in a single cuboid, but I'm not sure if it's better to pass in the shape explicitly.

I'll go ahead and proceed with passing the shape explicitly. Also, were you able to take a look at the additions to curation.py to see how that could be tested using PyTest?

Also, I am getting this error while trying to do pip install -e .[dev] on cellfinder-core package
error: Support for editable installs via PEP 660 was recently introduced
in setuptools. If you are seeing this error, please report to:

  https://github.com/pypa/setuptools/issues

  Meanwhile you can try the legacy behavior by setting an
  environment variable and trying to install again:

  SETUPTOOLS_ENABLE_FEATURES="legacy-editable"
  [end of output]

note: This error originates from a subprocess, and is likely not a problem with pip.
ERROR: Failed building editable for cellfinder-core
Failed to build cellfinder-core
ERROR: Could not build wheels for cellfinder-core, which is required to install pyproject.toml-based projects

[adamltyson]

This seems to be a common issue, but I haven't been able to reproduce with cellfinder-core. What OS and Python/pip/setuptools versions are you using?

[adamltyson]

Also, were you able to take a look at the additions to curation.py to see how that could be tested using PyTest?

I haven't had chance to go through in much detail, but what I would check is:

• That the GUI is created appropriately
• That the GUI elements work as intended, e.g. changing a parameter does indeed update the values it's meant to
• Briefly that the end result is correct, i.e. the correct cubes are being extracted.

[MysticElephant]

This seems to be a common issue, but I haven't been able to reproduce with cellfinder-core. What OS and Python/pip/setuptools versions are you using?

I am using Windows 10 version 21H2 and setuptools version 65.0.2

[adamltyson]

@MysticElephant I'm not sure if this will fix it for you, but on a new Windows 10 machine, I got this up and running by updating pip and setuptools and then installing Microsoft C++ Build Tools.

[MysticElephant]

Installing Build Tools worked. now i have the dev build for cellfinder-core.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[MysticElephant]

Now I'm moving onto cellfinder-core and whenever i try to run cellfinder_train in the environment using the --continue-train flag, it gives me this error

PermissionError: [Errno 13] Unable to open file (unable to open file: name = 'C:\Users\REDACTED.cellfinder', errno = 13, error message = 'Permission denied', flags = 0, o_flags = 0)

I did pip install -e .[dev] on cellfinder-core btw
How would I solve this?

[adamltyson]

PermissionError: [Errno 13] Unable to open file (unable to open file: name = 'C:\Users\REDACTED.cellfinder', errno = 13, error message = 'Permission denied', flags = 0, o_flags = 0)

This looks like an issue on your local machine, that you don't have access to that directory. Can you run as administrator?

[MysticElephant]

I ran as administrator and it's still giving me the same error.

Just as an FYI, I do have another environment with a non dev version of cellfinder installed.
At this point, I'm going to create a new dev environment from scratch. After I do that, should I install the entire cellfinder package, then install the dev builds of cellfinder-napari and cellfinder-core?

[adamltyson]

At this point, I'm going to create a new dev environment from scratch.

That sounds like a good idea. Unfortunately I don't have much time atm to debug development installation issues.

After I do that, should I install the entire cellfinder package, then install the dev builds of cellfinder-napari and cellfinder-core?

I would just install dev builds of the software you want to work on. Both cellfinder and cellfinder-napari depend on cellfinder-core.

[MysticElephant]

At this point, I'm going to create a new dev environment from scratch.

That sounds like a good idea. Unfortunately I don't have much time atm to debug development installation issues.

After I do that, should I install the entire cellfinder package, then install the dev builds of cellfinder-napari and cellfinder-core?

I would just install dev builds of the software you want to work on. Both cellfinder and cellfinder-napari depend on cellfinder-core.

The error I get now is:
Traceback (most recent call last):

File "C:\Users\Miniconda\envs\brainglobe-dev\lib\site-packages\multiprocessing_logging.py", line 78, in _receive
if self._is_closed and self.queue.empty():
File "C:\Users\Miniconda\envs\brainglobe-dev\lib\site-packages\multiprocessing_logging.py", line 78, in _receive
if self._is_closed and self.queue.empty():
File "C:\Users\Miniconda\envs\brainglobe-dev\lib\multiprocessing\queues.py", line 129, in empty
return not self._poll()
File "C:\Users\Miniconda\envs\brainglobe-dev\lib\multiprocessing\queues.py", line 129, in empty
return not self._poll()
File "C:\Users\Miniconda\envs\brainglobe-dev\lib\multiprocessing\connection.py", line 260, in poll
self._check_closed()
File "C:\Users\Miniconda\envs\brainglobe-dev\lib\multiprocessing\connection.py", line 260, in poll
self._check_closed()
File "C:\Users\Miniconda\envs\brainglobe-dev\lib\multiprocessing\connection.py", line 141, in _check_closed
raise OSError("handle is closed")
File "C:\Users\Miniconda\envs\brainglobe-dev\lib\multiprocessing\connection.py", line 141, in _check_closed
raise OSError("handle is closed")
OSError: handle is closed
OSError: handle is closed

I reinstalled everything in a new environment and it happens whenever i run cellfinder_train with the --continue-training flag

[adamltyson]

I'm afraid I don't know the answer to that one. Could you try a different Python version?

[adamltyson]

Hi @MysticElephant. Is there anything we can do help with this feature? If you don't have the time currently, we can close the PR, but feel free to re-open later on.