Refactoring all project & adding two new operators top hat - #3
Conversation
|
@dprodanov What do you think? |
|
@edelgadoh Thanks. We'll see what Dimiter says about it. I just wanted to point you also to the ImageJ Ops project, where the greater community is focusing these sorts of efforts. We haven't integrated grayscale morphology algorithms into Ops yet, but we definitely want to. See also imagej/imagej-ops#62 |
|
ok, thanks for the information about the other project, in my spare type I could help a little :) |
|
I saw the message just now. My bad. Sorry. |
|
The refactoring looks logical. |
|
Aahh, sorry @edelgadoh that this sat unmerged for so long! @dprodanov How do you want to proceed? Do you want to maintain this component? I added you as a collaborator to this repository. Or are you too busy? Please see https://imagej.net/Team for info about the team roles for SciJava-, ImageJ- and Fiji-related projects. @edelgadoh I know it has been over 3 years, but... do you still want to see this merged? If so, I can clean it up. Or if you don't care, we can just close it. |
|
@ctrueden Hi Curtis, |
|
@ctrueden Hi! yeah, it was three years ago hehehe, I just watch this conflicts, I'm going to resolve this. |
|
I can't do anything :( "This branch has conflicts that must be resolved |
@edelgadoh why not do that? Just click on the link " Use the command line to resolve conflicts before continuing." below to see detailed instructions how to do that. |
edelgadoh
left a comment
edelgadoh
left a comment
There was a problem hiding this comment.
basically I renamed a class and separated a big class in many classes
|
@dscho @dprodanov is there anything to do? or is it already ok? |
|
Have you tested the code? I see that it compiles but there is a conflict in MorphoProcessor (see below). |
|
@edelgadoh even if @dprodanov did not say it, it would be better to use @dprodanov I see you still stick to your terse replies, terse to the point of not exactly being helpful... I kind of hoped that several Google Summer of Code seasons would have helped you develop some mentoring skills, but I see that there is still quite some room for improvement. Having said that, I still think you have the capability to learn that. I told you back when I was still active in the Fiji project, and I tell you now: you got it in you. Discover that mentor in you! |
|
Hi Johannes,
You have no right to criticize my style.
On a more professional tone: Thanks for the suggestion for the rebasing. I am not very much versed in git.
When someone proposes a total rewrite of the code it is his onus to prove utility. Not mine. This has nothing to do with mentoring.
kind regards,
Dimiter
…On Sun, Jan 12, 2020 at 7:07 PM Johannes Schindelin < ***@***.***> wrote:
@edelgadoh <https://github.com/edelgadoh> even if @dprodanov
<https://github.com/dprodanov> did not say it, it would be better to use git
rebase <https://git-scm.com/docs/git-rebase> to rebase your changes on
top of the current tip of master, i.e. git rebase
b25846e (you might need to git fetch
https://github.com/fiji/Gray_Morphology/ master first).
@dprodanov <https://github.com/dprodanov> I see you still stick to your
terse replies, terse to the point of not exactly being helpful... I kind of
hoped that several Google Summer of Code seasons would have helped you
develop some mentoring skills, but I see that there is still quite some
room for improvement. Having said that, I still think you have the
capability to learn that. I told you back when I was still active in the
Fiji project, and I tell you now: you got it in you. Discover that mentor
in you!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3?email_source=notifications&email_token=AANM2OI6VUHRQA3BOUAYAVDQ5NL5XA5CNFSM4CJDOIG2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIXAILA#issuecomment-573441068>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AANM2OJJ4BAJVZ2UVFXC57TQ5NL5XANCNFSM4CJDOIGQ>
.
|
|
@dscho @dprodanov I just did the rebase with the master, it compiles and I already tested the generated JAR in the last version of ImageJ, It works! ... the two operators that I added worked too. |
|
Hi Edwin, Dimiter |
|
Dear @edelgadoh, did you look at the images? best regards, Dimiter |
|
Hi @dprodanov , I downloaded your images and compared with the JAR generated from your master branch https://github.com/fiji/Gray_Morphology and also with the JAR generated with the last commit from my fork https://github.com/edelgadoh/Gray_Morphology (and the current PR) and the images were identical. I used the ImageJ software to do a difference between each of this images and the result was an empty image, all pixels with zero value. I used the open operator with circular SE 4 and then changing with circular SE 8 blobs-se8-comment-github blobs-se8-branch-master blobs-se8-branch-forked blobs-se8-jar-downloaded blobs-se4-jar-downloaded blobs-se4-comment-github blobs-se4-branch-master blobs-se4-branch-forked |
|
I didn't find the difference, remember that I did a rebase with master and I only moved code, I didn't change any behavior. Basically I separated a big class in many classes and I added two operators. |
|
@dprodanov is there something I can do to continue with it? ... I didn't find any difference. |
4 participants
Hi all,
I did a refactoring in all the code. I created classes for each operator.
Finally I added two new operators "White Top Hat" & "Black Top Hat"
See you!