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

Add KatzCentrality plugin. #231

Closed
wants to merge 3 commits into from
Closed

Conversation

yossisp
Copy link

@yossisp yossisp commented Oct 16, 2021

The plugin adds Katz Centrality calculation in Statistics module.

@mbastian
Copy link
Member

mbastian commented May 8, 2022

Hi @yossisp I would like to propose some changes to your PR in light of the recent Gephi upgrades. In this PR, could you enable my contributions please, via https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

Thank you!

@yossisp
Copy link
Author

yossisp commented Jul 2, 2022

Hi @yossisp I would like to propose some changes to your PR in light of the recent Gephi upgrades. In this PR, could you enable my contributions please, via https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

Thank you!

Hi @mbastian, sure no problem, I see that "Allow edits by maintainer" checkbox is checked, please let me know if you have any issues adding your commits.

@mbastian
Copy link
Member

mbastian commented Jul 2, 2022

Thanks @yossisp! Unfortunately it seems that your branch is a fork from master-forge instead of master. Normally there should only be your module in there. Therefore I created a new branch with your code directly on this repository. I've made some updates to make the plugin work for the recent Gephi version. You can check it out and see if it works fine for you too.

@mbastian
Copy link
Member

mbastian commented Jul 3, 2022

Ah also, I don't know if you had a chance to look at the alternative implementation on this other PR: #186

Your implementation relies on Matrices while this other doesn't. I'm afraid matrices would take a lot of memory but it might be faster.

@yossisp
Copy link
Author

yossisp commented Aug 6, 2022

Thanks @yossisp! Unfortunately it seems that your branch is a fork from master-forge instead of master. Normally there should only be your module in there. Therefore I created a new branch with your code directly on this repository. I've made some updates to make the plugin work for the recent Gephi version. You can check it out and see if it works fine for you too.

Thanks @mbastian ! I used the katz centrality nbm file from the branch you created and it seems fine, but I also want to check with the university I developed the plugin for that they also confirm everything works as expected. Will update you as soon as I get their feedback. The only thing is that I get an error when running the plugin via mvn org.gephi:gephi-maven-plugin:run same error as described here I also use Mac. It happened now that I have to use JDK 11 for Gephi.

@yossisp
Copy link
Author

yossisp commented Aug 6, 2022

@mbastian

Ah also, I don't know if you had a chance to look at the alternative implementation on this other PR: #186

Your implementation relies on Matrices while this other doesn't. I'm afraid matrices would take a lot of memory but it might be faster.

I remember I saw this pr however I'm not sure it even works. Its author didn't respond since 2019. As far as I know the plugin I developed is used by the university which ordered this development and is not causing any memory issues.

@mbastian
Copy link
Member

mbastian commented Sep 2, 2022

Thanks @yossisp I merged manually from the katz-centrality-plugin branch. Your plugin is now published officially: https://gephi.org/plugins/#/plugin/katz-centrality.

If you want to make some changes, you can do that directly from that branch instead of your fork.

@mbastian mbastian closed this Sep 2, 2022
@mbastian mbastian mentioned this pull request Sep 2, 2022
@yossisp
Copy link
Author

yossisp commented Sep 8, 2022

Thanks @yossisp I merged manually from the katz-centrality-plugin branch. Your plugin is now published officially: https://gephi.org/plugins/#/plugin/katz-centrality.

If you want to make some changes, you can do that directly from that branch instead of your fork.

@mbastian Thanks for publishing! I have a few questions:

  1. Was the code merged to master branch? I don't see it in the master branch of gephi-plugins repo. I also see that the branch is 8 commits ahead of master.
  2. When I go to the plugin page and click on the "Source code" I get "File not found" error.
  3. The images in the plugin page are broken can they be fixed?

@yossisp yossisp reopened this Sep 8, 2022
@yossisp yossisp closed this Sep 8, 2022
@mbastian
Copy link
Member

mbastian commented Sep 9, 2022

@mbastian Thanks for publishing! I have a few questions:

  1. Was the code merged to master branch? I don't see it in the master branch of gephi-plugins repo. I also see that the branch is 8 commits ahead of master.

The master branch is the template branch for new plugins but the master-forge branch is actually where all the plugins are merged. So one needs to compare to this branch.

  1. When I go to the plugin page and click on the "Source code" I get "File not found" error.

Yes, you can fix this by changing this line. Make sure to also increment your plugin version otherwise changes are not taken in account.

  1. The images in the plugin page are broken can they be fixed?

You use relative paths in your README. Change that to absolute paths and it should work.

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.

None yet

2 participants