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

Support cex and bg #50

Merged
merged 5 commits into from
Jul 29, 2023
Merged

Support cex and bg #50

merged 5 commits into from
Jul 29, 2023

Conversation

grantmcdermott
Copy link
Owner

@grantmcdermott grantmcdermott commented Jul 28, 2023

Closes #48. Closes #49.

In addition to specifying these parameters manually, this PR also permits bg = "by" (similar to existing "by" keyword support for lty and pch).

I haven't added cex = "by" keword support yet, because I need to solve the potential problem of dual legend placement, e.g. one legend for colors and another for cex/size. However, once that's been cracked then it should be fairly trivial to support "bubble" plot types (c.f., #29).

Quick example of what this PR allows:

library(plot2)

palette("Tableau 10")
palette(adjustcolor(palette() , 0.3))

plot2(
    Sepal.Length ~ Petal.Length | Species, iris,
    pch = 21,
    col = "black",
    bg = "by",
    cex = 2
)

Created on 2023-07-28 with reprex v2.0.2

@vincentarelbundock
Copy link
Collaborator

Looks fantastic! Well done!

Two examples:

library(plot2)

palette("Tableau 10")
palette(adjustcolor(palette() , 0.3))

# Should `col` accept "by"?
plot2(
    Sepal.Length ~ Petal.Length | Species, iris,
    pch = 21,
    col = "by",
)
# Error in plot.xy(xy.coords(x, y), type = type, ...): invalid color name 'by'

# Default looks really pale on my screen
plot2(Petal.Length ~ Sepal.Length | Species, iris)

R/plot2.R Outdated Show resolved Hide resolved
R/plot2.R Outdated Show resolved Hide resolved
R/plot2.R Outdated Show resolved Hide resolved
@vincentarelbundock
Copy link
Collaborator

Looks really great overall. I left some comments. Once you've made decisions on those, I won't have anything else to add and think it's good to merge.

@grantmcdermott
Copy link
Owner Author

Super, thanks @vincentarelbundock.

Minor aside RE the paleness of this palette. That should be resolved by selecting a solid point type (e.g., pch = 19). Or, by keeping the filled point type (pch = 21) but explicitly selecting some fill (e.g. bg = "by" / bg = "grey", etc.)

@grantmcdermott grantmcdermott merged commit fde9a65 into main Jul 29, 2023
5 checks passed
@grantmcdermott grantmcdermott deleted the cex branch July 29, 2023 00:59
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.

bg arg not working cex arg not working
2 participants