search

paulbrodersen / matplotlib_venn_wordcloud

Venn diagrams with word clouds
MIT License
50 stars 12 forks source link

Updated README.md to reflect changes I added to examples.py. I added … #1

Closed jasonschray closed 7 years ago

jasonschray commented 7 years ago

I updated the examples.py file to show the plots of each example and then I added an if statement so that users can just enter:

python examples.py

to test out the examples. I really like your library here and while I struggled for a while before realized I needed to do plt.show() after each venn function, I still really like it and want to make it easier for other people that might make the same mistake I did for a while.

Thanks for the amazing work you did on this library!

paulbrodersen commented 7 years ago

HI Jason, thanks for the pull request. I like the idea of adding a script to examples.py such that the examples are simply run with 

python examples.py

Could you make a few changes to the PR before I merge it into master? Specifically:

1) Could you make the link to examples.py a relative link, that links to the file in the folder, instead of linking to your repo? Changing the link to 

[examples.py](./matplotlib_venn_wordcloud/examples.py)

should do it.

2) Regarding plt.show(). Not forcing the display of plots is default behaviour in matplotlib and I don't want to move away from that because I think there are good reason for not forcing the display (e.g. somebody might not have an interactive backend installed, in which case that command will either throw an error or simply halt the code execution without any feedback). Hence I would rather not have a plt.show() inside of any function body, which I would hence ask you to remove. That being said, I don't think there is any harm in having it at the end of the script that you added to examples.py.

3) If you add a script to plot the examples, then let's plot all examples by default. Together with my comment in 2) the script in examples.py becomes: 

if __name__ == "__main__":

    ex1()
    ex2()
    ex3()
    plt.show()

All of these changes are super nitpicky, but I like to keep my code bases clean. ;-) Let me know what you think. Out of curiosity, what are you using the word clouds for?

jasonschray commented 7 years ago

I think that all of the changes make alot of sense. I am still a novice programmer so I really appreciate your perspective. I am using them primarily to plot surface proteins in different conditions.

I wish I could think of a way or figure out a way to make a word cloud that is more structured and ordered to display lists so that I could make the venn diagrams better but alas I cannot figure it out. That said this is super useful for the plotting of protein groups on venn diagrams. Just like one of your examples actually.

I just finished making the changes you recommended. Let me know if you have any other thoughts.

paulbrodersen commented 7 years ago

Thanks for making the changes; looks great. If you have any more ideas, let me know!

I wish I could think of a way or figure out a way to make a word cloud that is more structured and ordered to display lists so that I could make the venn diagrams better but alas I cannot figure it out.

Are you referring to the API, i.e. the way that you provide the input to the functions, or the output, i.e. the image? I am open to changes at either end of the pipeline.