search

mbojan / alluvial

Alluvial diagrams
Other
139 stars 26 forks source link

add option to include (or omit) blocks from ribbon bundles #14

Closed corybrunson closed 8 years ago

corybrunson commented 8 years ago

Hi, i'm tinkering with your package for my own purposes and plan to PR what i think to be the most generally desirable changes. This one adds a logical "blocks" parameter that, when false, uses splines of the same style as the connector ribbons to link the connectors through the category columns. Thanks a lot for making the package in the first place!

mbojan commented 8 years ago

Hi. Thanks for this. The plot with blocks=FALSE is very nice.

Let me know what kind of features do you need. I would like to do a substantial rewrite in the future.

Before I merge this PR can you please:

  1. Add the example code from examples/blocks.R into examples/alluvial.R.
  2. Run devtools::document() to update alluvial.Rd.
corybrunson commented 8 years ago

Done (check pending)!

I'm moderately familiar with git at this point, but unfamiliar with collaboration on GitHub. Is it standard practice to run checks and documentation before making a pull request? (Now that i think of it, it does seem natural.) Also (and also for future reference), shall i mention the other features i have in mind here or at the Issues tab?

mbojan commented 8 years ago

Thanks!

After a PR a package should be complete, i.e. include both code and documentation. So if you extend it by adding an argument to a function, the PR should contain both code changes and documentation changes.

As for the features, I am interested what you have in mind. Feel free to file a separate issue for each feature.

corybrunson commented 8 years ago

OK, i'll do that. If i then make such a change myself, i'll check the relevant thread before submitting a PR. Thanks a lot.