Skip to content

introduced scale_position_*(reverse=True) #254

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

Closed
wants to merge 1 commit into from

Conversation

TyberiusPrime
Copy link
Contributor

This introduces a parameter 'reverse' to scale_x/y_continuous and scale_x/y_discrete,
which turns the scale around.

This is similar to scale_x/y_reverse, but works on transformed and discrete scales,
and fixes #253.

Both were doable before, just not easily - you'd have to define your own transformation,
and turn your discrete data into a Categorical with reversed categories.

I believe this hits the right abstraction and keeps compatibility with ggplot2.

scale_x/y_reverse are now wrappers around scale_x/y_continuous(reverse=True, *args, **kwargs).

For the continuous case, this is 'easy'
we simply define a transformation based on the existing one with * -1 at the right places and fiddle with the limits a bit to generate the same breaks.
reverse_transform() function should probably move to mizani though. @has2k1 you're in a better position to make this happen than I am.

For the discrete case this original looked like an easy case of reversing the mapping in scale_position.discrete for both discrete and continuous data, but it turns out that is being called twice from draw and that was relying on the continuous data being identity mapped.
The flow was like this:

  • map discrete -> mapped continuous
  • setup_data of geoms using mapped continuous data
  • map (now continuous) -> no op

This causes issues when you add in a geom that has continuous data - for example a hline,
since that data needs mapping, but only once (or we'd be back at the start again).

I think I've nailed it with a guard in layout.map_position - if not we're lacking a test case to expose it...

I've also included the doc changes suggested in #253 (and of course the doc for reverse=)

@has2k1
Copy link
Owner

has2k1 commented Jan 30, 2019

Instead of fiddling around with the scales to get the desired result. The solution is reversing the coordinate system. I have fixed the bug in e6ad834 so that coord_trans(x='reverse') now works.

@has2k1 has2k1 closed this Jan 30, 2019
@TyberiusPrime
Copy link
Contributor Author

That's cool.

Do you want scale_y_reverse to warn if the user sets a trans, to raise, or to create a reversed scale?

I'd also like to amend their annotation to point the user to coord_trans, if such a PR would be welcome

@has2k1
Copy link
Owner

has2k1 commented Jan 30, 2019

Do you want scale_y_reverse to warn if the user sets a trans, to raise, or to create a reversed scale?

It needs to be done for a bunch of scales_*, I thought about doing it while fixing this issue but could not come up with a nice and tidy way.

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.

scale_y_reverse and transformations does not reverse
2 participants