Raise error in skeletonize for invalid value to method param

[rum1887]

• Added an elif condition to check for the validity of the method passed.
• Added a unit test

Fixes #6661

Description

Checklist

• Docstrings for all functions
• Gallery example in ./doc/examples (new features only)
• Benchmark in ./benchmarks, if your changes aren't covered by an
  existing benchmark
• Unit tests
• Clean style in the spirit of PEP8
• Descriptive commit messages (see below)

For reviewers

• Check that the PR title is short, concise, and will make sense 1 year
  later.
• Check that new functions are imported in corresponding __init__.py.
• Check that new features, API changes, and deprecations are mentioned in
  doc/release/release_dev.rst.
• There is a bot to help automate backporting a PR to an older branch. For
  example, to backport to v0.19.x after merging, add the following in a PR
  comment: @meeseeksdev backport to v0.19.x
• To run benchmarks on a PR, add the run-benchmark label. To rerun, the label
  can be removed and then added again. The benchmark output can be checked in
  the "Actions" tab.

[jni]

Hi @rum1887, I've made a suggestion to comply with PEP8 formatting, and also tweak the wording a little.

Also, could you add a test for this? You can see an example of how to do this here:

scikit-image/skimage/morphology/tests/test_skeletonize.py

Lines 25 to 28 in d2f8e47

	def test_skeletonize_wrong_dim2(self):
	im = np.zeros((5, 5, 5))
	with pytest.raises(ValueError):
	skeletonize(im, method='zhang')

You should be able to copy those lines and adapt them to test this error message is raised.

[rum1887]

Hi @rum1887, I've made a suggestion to comply with PEP8 formatting, and also tweak the wording a little.

Also, could you add a test for this? You can see an example of how to do this here:

scikit-image/skimage/morphology/tests/test_skeletonize.py

Lines 25 to 28 in d2f8e47

	def test_skeletonize_wrong_dim2(self):
	im = np.zeros((5, 5, 5))
	with pytest.raises(ValueError):
	skeletonize(im, method='zhang')

You should be able to copy those lines and adapt them to test this error message is raised.

Hi @rum1887, I've made a suggestion to comply with PEP8 formatting, and also tweak the wording a little.

Also, could you add a test for this? You can see an example of how to do this here:

scikit-image/skimage/morphology/tests/test_skeletonize.py

Lines 25 to 28 in d2f8e47

	def test_skeletonize_wrong_dim2(self):
	im = np.zeros((5, 5, 5))
	with pytest.raises(ValueError):
	skeletonize(im, method='zhang')

You should be able to copy those lines and adapt them to test this error message is raised.

yes, on it!

[rum1887]

Hi @rum1887, I've made a suggestion to comply with PEP8 formatting, and also tweak the wording a little.

Also, could you add a test for this? You can see an example of how to do this here:

scikit-image/skimage/morphology/tests/test_skeletonize.py

Lines 25 to 28 in d2f8e47

	def test_skeletonize_wrong_dim2(self):
	im = np.zeros((5, 5, 5))
	with pytest.raises(ValueError):
	skeletonize(im, method='zhang')

You should be able to copy those lines and adapt them to test this error message is raised.

Hi @jni , I have added a unit test

[jni]

This is ready imho! Thank you @rum1887! 😊

[lagru]

Hey @rum1887, I noticed that another applicant was working on this in #6798 before you. I think they started on this in #6793 which was already closed when you created this one and than continued their work in #6798.

I am assuming that there was a misunderstanding and some confusion due to that? In the future, please take care to check if there is any previous work on this, especially by a fellow Outreachy applicant. And if there is, please feel welcome to work together and review each others contributions! There should be enough work for everyone.

@arsh73552, I'm sorry for the situation. It seems like this PR is much farther along.

@mkcor, @jni how should we handle this situation to be fair for each applicant? Considering that their solutions were quite similar, should we include @arsh73552 as a co-author so that both can record their contribution?

[arsh73552]

Ahh, I see.. It's ok I'll find other issues to work on.

[rum1887]

I'm sorry :(

I realize now that I should have reviewed the previous work before working on the issue.

The PR was closed, so I took up the issue!

Will ask before taking up an issue henceforth and if there's previous work on the issue, will review it instead of creating a new pr.

I don't mind if @arsh73552 PR is merged

[jni]

I like the idea of co-authorship, thank you for suggesting it @lagru — do you mind doing that in the merge commit message? I think you'll need to grab another commit from @arsh73552 to get the right Co-authored by: author-name <email> combo.

All good @rum1887, and sorry everyone that I also missed the other PR. It's a busy time with all the Outreachy contributions! 😅 Thanks all! 🙏

[arsh73552]

Thanks, that sounds great to me! I've created a PR to help with the same. (#6823)

[lagru]

Thanks everyone for being so understanding! 🙏 I don't think anyone wanted to step on anyone's toes and as @jni said, it's a bit busier than usual and it can be hard to keep track of something even during more quiet times. 😅

I'll merge once the CI is green and include @arsh73552 as a co-author. 🚀

[lagru]

Thanks everyone! @rum1887 and @arsh73552 you are both included in 4968b71 as authors. :)