Added average grain width measurement to disordered tracing #999
Conversation
Tests using [`pytest-regtest-2.3.2`](https://pypi.org/project/pytest-regtest/#history) are [failing on OSX](https://github.com/AFM-SPM/TopoStats/actions/runs/11780199328/job/32810287345?pr=988) pegging version to `pytest-regtest==2.3.1` to see if this resolves the issue.
There was a problem hiding this comment.
Thanks for tackling this @tcatley appreciated.
The tests are failing because you've added in the grain_width column to the resulting output files.
In order to update the regression test files you will have to install the tests dependencies and whilst at it may as well install the dev dependencies then you'll have the ability to run pre-commit locally.
pip install -e .[tests,dev]To install pre-commit you can then...
pre-commit installTo update the test files so they include the additional field you then...
pytest --regtest-reset tests/test_processing.pyThis will update files in the tests/_regtests_outputs/. You can add this to git with...
git add -u
git commit -m "tests(processing): Update regression test targets to include dna width"
git pushThis will add the updated files, make a commit with the above message and push to the branch here on GitHub (it will be included in this Pull Request automagically).
Tests
Ideally we should have a test in place to check the method returns the correct values in isolation from the global regression test.
This involves adding a test to tests/tracing/test_disordered_tracing.py but if you look through you will see many are marked as skipped as they are awaiting being written.
I can write this up in the existing issue (or create a new one) if you don't fancy having a stab at it.
|
Thanks for the quick update/fixes @tcatley almost there but your second commit #09e10d1 change the column name to Thus you need to run the regeneration of the regression test files again. We can avoid duplicating commit messages by ammending and force pushing... pytest --regtest-reset tests/test_processing.py
git add -u
git commit --amend
git push --force |
…h and improved docstring
tcatley
force-pushed
the
tcatley/dna-width
branch
from
09e10d1 to
3ed5d53
Compare
|
Hi @tcatley, Thanks for addressing the changes. Unfortunately the tests still fail, but in a different location (this is progress!!! 😁 ). Here its Unfortunately these checks are made outside of the |
|
Great, thanks @ns-rse! Let me know if you need anything else from me |
Splits `test_trace_image_disordered` into two tests, the original compares dictionaries, the newer (suffixed with `_dataframes`) compares the dataframes/csv files that are produced, of which there are two and one needed updating to include the `grain_width_mean` column that #999 introduces.
As [suggested](#996 (comment)) by @SylviaWhittle we could all do with an _aide memoire_ when making Pull Requests that we have covered everything we should. This Pull Request therefore introduces just such a template to aid contributors in making sure we have covered everything. --- - [ ] Existing tests pass. **N/A** no changes to code base introduced. - [ ] Documentation has been updated and builds. **N/A** no changes to code base introduced. - [x] Pre-commit checks pass. - [ ] New functions/methods have typehints and docstrings. **N/A** no changes to code base introduced. - [ ] New functions/methods have tests which check the intended behaviour is correct. **N/A** no changes to code base introduced.
Co-authored-by: Sylvia Whittle <[email protected]>
…h and improved docstring
Splits `test_trace_image_disordered` into two tests, the original compares dictionaries, the newer (suffixed with `_dataframes`) compares the dataframes/csv files that are produced, of which there are two and one needed updating to include the `grain_width_mean` column that #999 introduces.
Two final tests needed updating to account for new `grain_width_mean` columns. Also required rebasing onto `main` and `ns-rse/994-unpin-topoly` so the changes look larger than they might.
|
It was a bit more involved than I thought @tcatley and involved another set of regression tests being updated and rebasing onto existing branches that are out for pull requests (#995) which fix the tests in light of a newer version of #1001 looks big but that is a consequence of "rebasing" which brings the changes from the other branches into yours.
|
Fixing all tests, required rebasing onto `main` and `ns-rse/994-unpin-topoly` to pull in updates to some unrelated tests.
|
For other reviewers... I split the failing test into two, one for the nested dictionaries that are loaded from pickles and one for comparing dataframes via I did some more reading on syrupy and think it might be a viable alternative to replace |
|
@tcatley already had a simple test which I've quickly implemented and expanded slightly. |
- `px2nm` > `pixel_to_nm_scaling` so this keyword is consistent across all functions/methods. - Removed an unnecessary comment line `# Code will go here` - Already import `ndimage` from `scipy` so prefixed `distance_transform_edit()` rather than importing it in isolation. I _haven't_ updated `docs/data_dictionary.md` with the new `grain_width_mean` field that is written to the output files and will include that in #1001.
Added function
calculate_dna_width()into classdisorderedTracewhich uses the smoothed mask and the pruned trace to calculate the average width of the mask using a distance transform. Outputs a float value of the width in metres to theall_statistics.csv.