corybrunson / ggalluvial

ggplot2 extension for alluvial plots
http://corybrunson.github.io/ggalluvial/
GNU General Public License v3.0
497 stars 34 forks source link

cool #1

Closed mbojan closed 5 years ago

mbojan commented 8 years ago

Hi! Great you picked that up! Pity you did not let me know! ;) It's on my TODO for alluvial since a year or so (https://github.com/mbojan/alluvial/issues/10).

corybrunson commented 8 years ago

Oh, i'm sorry! I'd looked through the issues on your package but didn't remember that one. I was planning to ask for your input once i felt comfortable with it, but your star beat me to it. I don't think my attempt is ideal, so any ideas you've had are very welcome. : )

corybrunson commented 8 years ago

I have another etiquette question for you, if you don't mind: It seems that yet another person already had this idea and initiated a repo of the same name as this, though no code has been pushed there yet. Would it be appropriate for me to open an issue there to say hi, mention this repo, and invite suggestions (or a completely different approach)?

mbojan commented 8 years ago

If you mean mjsmith037/ggalluvial I know he made a fork of alluvial couple of months ago. The repo is empty though, so there is not a lot to talk about...

I did some experiments for the ggplot2 version, but I have them in messy scripts. I'll try to sort them out in the coming days. There is quite some common code with the base graphics version so I think I'll push them as a branch of alluvial. I am not sure whether it is better to have it as a separate package or in alluvial. Having common code and not a lot of separate dependencies would be an argument to keep them together. I'm not sure.

Do you plan to port the alluvial_ts to ggplot framework too? Maybe it is a good occasion to come up with a more sexy name...

mbojan commented 8 years ago

BTW alluvial_ts was used here (http://biqdata.wyborcza.pl/ilu-polakow-mieszka-w-miastach). BiqData is a data journalism blog/service of Gazeta Wyborcza, which is the largest Polish daily newspaper. Congrats! :)

corybrunson commented 8 years ago

Nice going! My sense is that base graphics will always be better for polished or professional graphics, whereas ggplot2 is better for exploration and experimentation. Maybe that's another reason to put them in the same package. I'm not sure if the ggplot2 extensions library would prefer to have only the ggplot2 functions included, but if so then that would be a reason to keep them separate. It will be great to see what directions you've taken for ggplot2.

I did eventually want to adapt alluvial_ts() as well. Something on my list to try in the original alluvial is formula–data calls analogous to those of mosaicplot(), e.g.

In the original usage, any number of independent variables could be used; whereas in the panel usage, it would only be possible to have one independent variable and one dependent variable. My hope was to combine alluvial() and alluvial_ts() into one function with two methods, decided either by the formula or (if no formula is given) by some additional type argument, maybe. Would that make sense to you? (I don't know if something analogous could be done in ggplot2.)

corybrunson commented 7 years ago

There's a chance that i'll take another crack at the issues with this package (and an analog to alluvial_ts) in the next couple of months (i.e. this winter). After that, though, i probably won't have the time and incentive to improve it. Would you like to absorb it into alluvial then?

The only module i notice that both could in principle share is my zigzag function for layering the flows. It's not a separate module in yours, but i could send a pull request for that while i'm working on ggalluvial. Are there other modules you think both functions could build on?

mbojan commented 7 years ago

Thanks for the heads-up!

There's a chance that i'll take another crack at the issues with this package (and an analog to alluvial_ts) in the next couple of months (i.e. this winter). After that, though, i probably won't have the time and incentive to improve it. Would you like to absorb it into alluvial then?

Sure.

The only module i notice that both could in principle share is my zigzag function for layering the flows. It's not a separate module in yours, but i could send a pull request for that while i'm working on ggalluvial. Are there other modules you think both functions could build on?

Yeah. alluvial() is still a bit of spaghetti code. I had different idea about the interface, also for a ggplot-based version, but I don't have too much experience with ggplot2 internals so maybe that would be anyway unachievable. Definitely a good geom is already a lot. I'll think about other ideas/extensions and let you know.

corybrunson commented 7 years ago

Hey! I made the fixes i've wanted to for a while, specifically

So i feel OK with this, though i still don't care much for the way axes are assigned. I'm also thinking that the two sets of functions make more sense as separate packages. Currently ggalluvial imports alluvial, but only for the Refugees dataset; though, if alluvial were modularized (e.g. with zigzag() and other flow-layering options), then the modules might better be kept there and imported here, another reason to combine them. But another reason to keep them separate is to make ggalluvial a standalone ggplot2 extension. What do you think? And, do you have further immediate plans for alluvial?

corybrunson commented 5 years ago

It may still make sense to move some of the internals into a separate package for processing data in preparation for both packages (and potentially others), but that should probably be discussed in a separate issue.