TopoStats icon indicating copy to clipboard operation
TopoStats copied to clipboard

Optimized Area thresholding

Open AuroVarat opened this issue 1 year ago • 5 comments

This modification enhances the area thresholding function, significantly improving processing speed, especially for large images.

AuroVarat avatar Oct 25 '24 11:10 AuroVarat

Thanks for the Pull Request @AuroVarat I'll take a look through this on Monday.

In the mean time would you be able to address the failing tests please?

If you're not familiar with tests the software development section of our docs has some advice. You'll want to pip install -e .[dev,tests.docs] and ideally install the pre-commit hooks with pre-commit install. We use the pytest-testmon extension and so you'd have to run the tests once to check everything is working with pytest --testmon and then when you make commits only the tests that are affected will run.

Note also that pre-commit.ci has modified this branch by fixing some minor linting errors so you may want to git pull to get those changes (although if you install pre-commit locally it will make them before you can make your next commit again).

ns-rse avatar Oct 25 '24 14:10 ns-rse

Hi, I am not familiar with tests but I managed to go through the steps and do the "pytest --testmon". It fails 8 tests, but it fails 8 tests regardless of my changes ( I did the same test by running it on the current main branch). Maybe I am doing something wrong, I will look at it when I have more time.

But if you see my changes, they are minimal and should not result in this failure. The code seems to be working fine when I am doing my analysis.

Thanks.

AuroVarat avatar Nov 12 '24 17:11 AuroVarat

@ns-rse Hi Neil, Hope you are well. Were you able to merge it? If not, I can look into the test failures again.

AuroVarat avatar Jan 20 '25 13:01 AuroVarat

Hi @AuroVarat

Apologies I've not had time to investigate why the tests are failing as I'm not full time on this project and have had various other tasks to work through.

The logs show that there are two elements that are different in the revised code. How big an issue that might be isn't something I can really answer but we have tests in place to ensure that changes under the hood don't change the expected results. You could get more information on where these have changed running the specific test with...

pytest -vv tests/test_grains.py::test_find_grains

...although there may be other places where tests fail as we use the -x flag which exits the test suite on the first failure that is encountered so it would be worth running the whole test suite (this can be done in parallel if you install all of the dev optional dependencies in your virtual environment with pytest -n # where # is the number of cores to use which will depend on your system and what else you want to do whilst its running).

If you are new to testing you may find the following tips on Contributing useful as they show how to get setup with a development environment and install the pre-commit hooks that we have in place which performs various linting.

I've got some failing tests of my own to work through but feel free to message if you've any questions.

ns-rse avatar Jan 20 '25 14:01 ns-rse

Hi @ns-rse I managed to address the failing test_finding_grains check and all the rest seem to pass on my system allowing me to commit, once I pushed it seemed to fail at codespell.


codespell................................................................Failed
- hook id: codespell
- exit code: 65

docs/workflow.md:28: OT ==> TO, OF, OR, NOT, IT
docs/workflow.md:59: OT ==> TO, OF, OR, NOT, IT
docs/workflow.md:61: OT ==> TO, OF, OR, NOT, IT
3
0

AuroVarat avatar Feb 03 '25 16:02 AuroVarat