Added docstring to skimage.registration submodule. #6791
Conversation
skimage/registration/__init__.py
Outdated
| """Image registration module. | ||
| """ |
There was a problem hiding this comment.
Looks good, thank you for staring on this! :) What do you think about including a more verbose description on what functions in this module might be used for and what "image registration" is?
There was a problem hiding this comment.
That would look much better. I will surely add the suggested changes to my PR.
There was a problem hiding this comment.
I think i have messed up something while commiting.. Should i make another branch and create a new pull request?
There was a problem hiding this comment.
That's not a problem. A good opportunity to learn some git. 💪
I'm not sure how your local branch looks but the state of mahamtariq58:docstring-to-skimage.registration doesn't look that messed up. It's just a whitespace change that I actually agree with. 😄
You could just try to add the changes in 0a3d56e back to the module and create a new commit. When we merge, we usually squash all these different commits into a single commit.
In case your local branch doesn't reflect the remote state, you should be able to force the remote state with
git fetch --all
git reset --hard origin/docstring-to-skimage.registration
Let me know if that helps.
lagru
added
the
📄 type: Documentation
This reverts commit ecde1a4.
There was a problem hiding this comment.
Nice, @mahamtariq58, I think you managed to fix the commit history. 🎉 I left a few more comments regarding the docstring itself. Please, ask if something is unclear or you don't agree with the suggestion. :)
| Functions | ||
| --------- | ||
| phase_cross_correlation: function | ||
| Perform the phase correlation to determine the shift between two images using DFT upsampling. | ||
|
|
||
| optical_flow_tvl1: function | ||
| Estimates the optical flow components for each axis between two images using TV-L1 solver. | ||
|
|
||
| optical_flow_ilk: function | ||
| Estimates the optical flow between two images using iterative Lucas-Kanade(iLK) solver. |
There was a problem hiding this comment.
No need to list these functions in the docstring. In our HTML documentation, the docstring is followed by an automatically generated list. And IDEs usually make these contents discover-able through autocompletion.
There was a problem hiding this comment.
Now I get it where did I go wrong regarding the commits. Hopefully not to repeat them next time.
There was a problem hiding this comment.
Alright, do I need to add just the general purpose of these functions in the extended summary?
There was a problem hiding this comment.
Yeah, it would be good to help users who are discovering this module and are asking themselves "okay, restoration, what is it, why is it useful, and what is it used for".
| """ | ||
| Image registration module | ||
|
|
||
| This module provides a set of functions to register (align two images into single coordinate system) based on various transformation models. Implemented algorithms include phase_cross_correlation and optical flow estimator. |
There was a problem hiding this comment.
Could you wrap this to a max line length of 88?
| See Also | ||
| -------- | ||
| See the documentation for each function for details. | ||
|
|
||
| Notes | ||
| ----- | ||
| Color images are not supported for optical flow estimators. | ||
|
|
||
| Examples | ||
| -------- | ||
| >>> from skimage import data | ||
| >>> from skimage.data import stereo_motorcycle | ||
| >>> from skimage.registration import optical_flow_tvl1 | ||
| >>> image0, image1, disp =stereo_motorcycle() | ||
| >>> image0=rgb2gray(image0) | ||
| >>> image1=rgb2gray(image1) | ||
| >>> flow= optical_flow_tvl1(image1,image0) |
There was a problem hiding this comment.
I'm sorry to say this, but I also think that these sections don't provide much value or state things that are better placed in their respective docstrings.
| """ | ||
| Image registration module |
There was a problem hiding this comment.
Maybe we can make this line describe what image registration is? What do you think about
| """ | |
| Image registration module | |
| """Utilities to relate different sets of data in one coordinate system. |
There was a problem hiding this comment.
That would seem much better.
|
Hey, there hasn't been any activity on this pull request for more than 180 days. For now, we have marked it as "dormant" until there is some new activity. You are welcome to reach out to people by mentioning them here or on our forum if you need more feedback! Otherwise, we would be thankful for a short update, for example if you would like to continue or if you are okay with someone else doing so. In any case, thank you for your contributions so far! |
Description
Checklist
./doc/examples(new features only)./benchmarks, if your changes aren't covered by anexisting benchmark
For reviewers
later.
__init__.py.doc/release/release_dev.rst.example, to backport to v0.19.x after merging, add the following in a PR
comment:
@meeseeksdev backport to v0.19.xrun-benchmarklabel. To rerun, the labelcan be removed and then added again. The benchmark output can be checked in
the "Actions" tab.