Skip to content

Bradley Thresholding added#919

Closed
mailjenil wants to merge 2 commits intoscikit-image:masterfrom
mailjenil:brad
Closed

Bradley Thresholding added#919
mailjenil wants to merge 2 commits intoscikit-image:masterfrom
mailjenil:brad

Conversation

@mailjenil
Copy link
Copy Markdown

Hello all,

I have implemented Bradley Thresholding algorithm as cited in Adaptive Thresholding Using the Integral Image.(http://people.scs.carleton.ca/~roth/iit-publications-iti/docs/gerh-50002.pdf)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be an unnecessary indent

@RONNCC
Copy link
Copy Markdown
Contributor

RONNCC commented Mar 12, 2014

in the future running the build on your own machine would help before submitting a pull request ;), you just forgot quite a few imports here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

your indent for all of these is wrong. remember tab = 4 spaces, if you use vim this is expandtab=4, either way you should be able to set this on most text/code editors.

@mailjenil
Copy link
Copy Markdown
Author

@stefanv @RONNCC @vighneshbirodkar Thanks a lot for reviewing the code and pointing out obvious bugs in it.

It was my first contribution to scikit-image and so did a lot of blunders.

I will fix them as soon as possible.

@JDWarner
Copy link
Copy Markdown
Contributor

@mailjenil Everyone's PRs have comments! There's no shame in it, just a learning experience. Thanks for the PR!

@mailjenil
Copy link
Copy Markdown
Author

@JDWarner Learning is the best part of contribution I believe.

However, which editor will you recommend for PEP8 guidelines? I am currently using 'Geany' but missed out on PEP8 plugin part.

@JDWarner
Copy link
Copy Markdown
Contributor

There are various editor preferences and I don't want to start a war, but for what it's worth...

My personal preference - and I believe it is an excellent option - is Sublime Text. You can configure it to eliminate trailing whitespace on save, but for automatic PEP8 checking you can install the "flake8" package. I believe there's even an "autopep8" package which you can configure to run on save and fix errors automatically. That doesn't always work perfectly, though, so I prefer to have them flagged as I type and fix them manually. Over time it's better at encouraging best practices, as well.

You can set similar things up with other editors as well.

@mailjenil
Copy link
Copy Markdown
Author

@JDWarner Thanks for the guidance.I will check it out and see whether it works for me.

@RONNCC
Copy link
Copy Markdown
Contributor

RONNCC commented Mar 13, 2014

@mailjenil no matter how skilled of a coder you are, you'll probably have a comment somewhere :), code reviews are a hallmark of coding. In the future running it on your own machine before pulling might help, I also have import errors sometimes haha

@stefanv
Copy link
Copy Markdown
Member

stefanv commented Apr 8, 2014

Travis is failing.

@stefanv
Copy link
Copy Markdown
Member

stefanv commented May 5, 2014

Any update on this?

@mailjenil
Copy link
Copy Markdown
Author

I will soon fix it..
On May 6, 2014 5:10 AM, "Stefan van der Walt" notifications@github.com
wrote:

Any update on this?


Reply to this email directly or view it on GitHubhttps://github.com//pull/919#issuecomment-42253760
.

@ahojnnes
Copy link
Copy Markdown
Member

ahojnnes commented Jul 5, 2014

@mailjenil ping.

@stefanv
Copy link
Copy Markdown
Member

stefanv commented Nov 6, 2014

This PR is now more than 6 months overdue. Closing unless there is further discussion.

@mailjenil
Copy link
Copy Markdown
Author

Will sort it out in few days. Probably next weekend.
On Nov 7, 2014 5:13 AM, "Stefan van der Walt" notifications@github.com
wrote:

This PR is now more than 6 months overdue. Closing unless there is further
discussion.


Reply to this email directly or view it on GitHub
#919 (comment)
.

@sciunto sciunto self-assigned this Jul 14, 2016
@sciunto
Copy link
Copy Markdown
Member

sciunto commented Jul 14, 2016

Assigned to myself. I'll try to push it to the end.

@stefanv stefanv assigned alexdesiqueira and unassigned sciunto Apr 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants