Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update edge.py #112

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Update edge.py #112

wants to merge 10 commits into from

Conversation

SujoyKG
Copy link

@SujoyKG SujoyKG commented Apr 23, 2020

added edge detector based on Sujoy's algorithm

added edge detector based on Sujoy's algorithm
@luispedro
Copy link
Owner

Sorry for the delay, but this fails on travis (also, locally):

https://travis-ci.com/github/luispedro/mahotas/builds/170839189

@luispedro
Copy link
Owner

Also, this PR and #111 should be merged into one or does this one completely subsume the previous one?

@SujoyKG
Copy link
Author

SujoyKG commented Jun 11, 2020

Sorry for the delay, but this fails on travis (also, locally):

https://travis-ci.com/github/luispedro/mahotas/builds/170839189

But what's the error Luis? I am not getting/clear... Please help.

@SujoyKG
Copy link
Author

SujoyKG commented Jun 11, 2020

Also, this PR and #111 should be merged into one or does this one completely subsume the previous one?

These are the test cases

@luispedro
Copy link
Owner

The test cases should be with the code together in the same PR

But, in any case, the code does not even pass a smoke test: it mixes tabs & spaces.

@SujoyKG
Copy link
Author

SujoyKG commented Jun 11, 2020

The test cases should be with the code together in the same PR

But, in any case, the code does not even pass a smoke test: it mixes tabs & spaces.

I will correct the space/tab errors & resubmit. But I am not sure how to put both (test case + function) together in one PR. Can you please help Luis?

@luispedro
Copy link
Owner

You can update PRs by adding commits to their respective branches. It does not need to be a single commit per PR.

corrected space/tab errors... all indentation are now tabs...
@coveralls
Copy link

coveralls commented Jun 12, 2020

Coverage Status

Coverage increased (+0.04%) to 97.449% when pulling 57e5245 on SujoyKG:patch-2 into 382fb2a on luispedro:master.

update threshold 't'
Copy link
Author

@SujoyKG SujoyKG left a comment

Choose a reason for hiding this comment

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

This is final commit for function 'sujoy()'. Please accept changes.

SujoyKG added 5 commits June 13, 2020 12:26
test_edge.py updated with sujoy-edge-detector test cases
Patch 2 updated with test cases
updated test cases with parameter "kernel_nhood=1"
line 103 from"mh.sujoy" to "sujoy"
Copy link
Author

@SujoyKG SujoyKG left a comment

Choose a reason for hiding this comment

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

This is final commit for function 'test cases'. Please accept changes.

@SujoyKG
Copy link
Author

SujoyKG commented Jun 14, 2020

Hi Luis, "build" is complete on Travis. Added few more test-cases also. sorry for so many "commits"; as I am new to GITHUB. Thanks.

@SujoyKG
Copy link
Author

SujoyKG commented Oct 15, 2021

Is there anything wrong, that you are not merging, Luis?

@luispedro
Copy link
Owner

Is this approach being used? I only saw your manuscript, but I did not find some citations and/or comments from users.

@SujoyKG
Copy link
Author

SujoyKG commented Oct 23, 2021

I don't know who are using this. However this has got accepted in Julia Image Library (https://juliaimages.org/stable/examples/contours/sujoy_edge_demo/#Edge-detection-using-Sujoy-Filter) & also in the process of inclusion in a Java Image Library. Soon I shall copyright this. Thanks.

@SujoyKG SujoyKG closed this Dec 28, 2021
@SujoyKG SujoyKG deleted the patch-2 branch December 28, 2021 15:08
@SujoyKG SujoyKG restored the patch-2 branch January 6, 2022 15:06
@SujoyKG SujoyKG reopened this Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants