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

new visualizations #91

Merged
merged 8 commits into from Jan 15, 2021
Merged

new visualizations #91

merged 8 commits into from Jan 15, 2021

Conversation

bijae
Copy link
Contributor

@bijae bijae commented Dec 18, 2020

Hi,
I have implemented a number of visualizations in the BasicUsage file. Addionally, I did some minor changes (mainly typos) in some other files. As this is my first use of github, I do not know how to separate both topics and make two pull requests...
I hope this works out!

Added requirement in introduction
added utf-8 encoding in "open" to make example running
deleted import of scipy.misc.imresize
corrected typo in introduction
corrected typos in introduction
included various visualizations: starburst map, property distribution, quality heatmap and property plots using plotly library
@JustGlowing
Copy link
Owner

hi @bijae thanks for this nice contribution. I'll review it in the next few days.

@JustGlowing
Copy link
Owner

The charts are absolutely amazing, especially the Start Burst map!

However, I think they're a little bit too complicated for a basic usage documentation. What do you think about creating a new file (eg. AdvancedVisualizations.ipynb) with the new additions? It would also be nice to have a comment on each chart that describes how to read them.

@JustGlowing
Copy link
Owner

Also, I'd like you to know that I'm planning on changing the licence of Minisom to MIT or GPL. I hope it's not a problem.

@bijae
Copy link
Contributor Author

bijae commented Dec 23, 2020 via email

@JustGlowing
Copy link
Owner

Hi Birgit,

I want to publish an article about MiniSom on the Journal of Open source Software (https://joss.theoj.org/) but the current licence is not compatible with their policies.

Merry Christmas to you too 😄 🎄

corrected typo
advanced visualizations in new file
@bijae
Copy link
Contributor Author

bijae commented Jan 6, 2021 via email

@JustGlowing
Copy link
Owner

Hi Birgit,

Thanks for offering support with the publication but the paper is mostly done, I didn't submit it yet because I'm not sure if I want to go into the murky waters of changing the license.

It would be great if you could cite MiniSom as specified here: https://github.com/JustGlowing/minisom/#how-to-cite-minisom

Thanks for submitting the changes to the visualization examples. You don't have to do another Pull Request if you pushed the changes. I can see that the latest changes were done 10 days ago. Did you push anything more recently?

I hope you're starting the year in a good way!

@bijae
Copy link
Contributor Author

bijae commented Jan 6, 2021 via email

@JustGlowing
Copy link
Owner

hi @bijae thanks for this nice contribution. I'm going to merge it.

@JustGlowing JustGlowing merged commit f01f200 into JustGlowing:master Jan 15, 2021
JustGlowing added a commit that referenced this pull request Apr 26, 2021
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.

None yet

2 participants