gephi / gephi-plugins

Repository for Gephi Plugins maintained by the team. Each plugin has it's branch.
268 stars 620 forks source link

word cloud plugin for review and publication #294

Closed seinecle closed 1 year ago

seinecle commented 1 year ago

New plugin or plugin update?

What is the purpose of this plugin?

Showing a word cloud of a textual node attribute, while the user is hovering over the network with their mouse.

How to test your plugin in Gephi?

  1. Open a network with a textual attribute. A test network is provided here.
  2. Click on "refresh" in the plugin to get a list of the attributes. For the test, choose "description".
  3. Click on start and follow the instructions

Checklist before submission

mbastian commented 1 year ago

Thanks for addressing the comments. I added a few commits, check it out. I wanted to show you how to use Unit tests for your plugin, instead of creating main methods with hardcoded filenames.

I added a TopTermExtractorTest file with a simple test but I was not able to make it work. It produces an exception,

java.lang.NullPointerException
    at java.base/java.io.Reader.<init>(Reader.java:167)
    at java.base/java.io.InputStreamReader.<init>(InputStreamReader.java:109)
    at net.clementlevallois.umigon.tokenizer.controller.PatternOfInterestChecker.loadPatternsOfInterest(PatternOfInterestChecker.java:32)
    at net.clementlevallois.umigon.tokenizer.controller.UmigonTokenizer.tokenize(UmigonTokenizer.java:52)
    at net.clementlevallois.wordcloud.TopTermExtractor.tokenizeSelectedTextualAttributeForTheEntireGraph(TopTermExtractor.java:78)
    at net.clementlevallois.wordcloud.TopTermExtractorTest.testOnGraph(TopTermExtractorTest.java:16)

Not sure why this might only fail with the unit test while the plugin is working. In any event it's clear that the patterns_of_interest.txt file should not be located in the src/main/java folder but in the src/main/resources. That is what causes the exception I think.

Then, regarding design and testability. The fact you're using a static DataManager in this method is far from ideal. My advice when possible is to stick to method return types so it's really easy to test a method output.

seinecle commented 1 year ago

Hello, Merci pour le temps pris! Ok je ferai mes tests de cette facon, bien compris. J'ai mis a jour la dependence pour que les fichiers soient bien dans le repertoire ressources. Pour le DataManager, j'avoue que je suis un peu fatigue sur le refactoring la, mais est-ce que tu peux me dire quelle alternative tu as en tete : en faire une variable globale, non statique, dans le top component par exemple?

seinecle commented 1 year ago

I have added a readlockcall on the graph before the unreadlockmethod otherwise the test threw an exception

mbastian commented 1 year ago

Hello, Merci pour le temps pris! Ok je ferai mes tests de cette facon, bien compris. J'ai mis a jour la dependence pour que les fichiers soient bien dans le repertoire ressources. Pour le DataManager, j'avoue que je suis un peu fatigue sur le refactoring la, mais est-ce que tu peux me dire quelle alternative tu as en tete : en faire une variable globale, non statique, dans le top component par exemple?

No worries, it was more for future looking projects and learning how to do it. You can still add more tests if you want. This way, you'll also see by yourself that some patterns are easier to tests that others. No need to refactor the DataManager at this point. A good next step learning for you might be to dive in more in the concept of Model + Controller. Here, the DataManager stuff would be part of the Model.

I have added a readlock call on the graph before the unreadlock method otherwise the test threw an exception

Thanks, I replaced the readUnlock with a readUnlockAll. This one doesn't throw an exception and is good when we're not sure if we're holding a lock or not. That is the case here. If the loop finishes normally we don't hold a lock (the NodeIterable takes care of releasing it for us). If the loop gets cancelled, we hold a lock.