swarmx() and swarmy() fail with infinite inputs

[billdenney]

Related to eclarke/ggbeeswarm#87

Thanks for making this great package!

I recently started using it via ggbeeswarm, and I found that it gave an error for infinite inputs. I'd propose that infinite and NA values could be treated categorically and that they could then be processed by the package. I will propose a PR to make this occur.

[aroneklund]

Hi Bill. Thanks for posting this; it's an interesting idea.

The beeswarm function has had fairly stable behavior since 2011, so I'm disinclined to make radical changes. Stopping with an error when trying to plot infinite values has always been the intended behavior.

As I understand, what you are proposing is basically a built-in transformation of the data, where you map Inf, -Inf, and NA to values outside the finite range of the data. I suspect this would be intuitive to some users, but others might prefer a different behavior. Have you considered transforming your data before sending it to beeswarm?

[billdenney]

Thanks for the quick review.

I understand the goal of stability, and I don't want to break what is working.

The proposed PR doesn't map values for plotting to be plotted, if they wouldn't otherwise. The PR pushes the question of "what can be plotted?" to the plot function rather than making the decision (by an error) at the transformation step.

We can definitely modify the data before sending it to beeswarm, but I thought that others may benefit from the change.

[aroneklund]

Okay, I think I misunderstood you the first time, and it sounds like we're actually on the same page.

If you use beeswarm only to do the jittering (do.plot = FALSE, as is the case when calling from ggbeeswarm), then it would quietly jitter infinite and NA values as separate categories.

If you use beeswarm to generate plots (do.plot = TRUE, the default), then it would continue to throw an error if there are infinite values, as does stripchart.

Right? If so, I don't think this change would require a new argument.

Is this what your PR does? I tried to read it, but I think you lost me when you rewrote code that wasn't broken. 😉

[billdenney]

Yes, your interpretation is correct.

Whoops for changing what wasn't broken. I was trying to learn how the code worked, saw duplication and tried to remove it. Would you prefer that I make this a smaller PR that doesn't create the swarmboth() function?

[billdenney]

@aroneklund, I made a smaller PR that does not consolidate the two functions.