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

Rely on fill instead of color for graphr/s/t functions for flexibility and consistency #46

Open
henriquesposito opened this issue Dec 21, 2023 · 4 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@henriquesposito
Copy link
Contributor

henriquesposito commented Dec 21, 2023

Currently we rely on the color aesthetics to color the inside of nodes. However, once node colors and shapes are declared, color becomes the outside color.

For example, autographr(ison_lawfirm, node_color = "school")and autographr(ison_lawfirm, node_color = "school", node_shape = "practice").

This happens because of inconsistencies in using both ggplot2 and ggraph packages in our functions. By refactoring and making code more consistent, will allow that we use color for the outside and fill for inside colors throughout. That is, we will have the "node_color" and "node_fill" arguments in the autographr/s/d functions.

@henriquesposito henriquesposito added bug Something isn't working enhancement New feature or request labels Dec 21, 2023
@henriquesposito henriquesposito changed the title Rely on fill instead of color for autograph r/s/d functions for flexibility and consistency Rely on fill instead of color for autographr/s/d functions for flexibility and consistency Dec 21, 2023
@henriquesposito henriquesposito self-assigned this Jan 15, 2024
@henriquesposito
Copy link
Contributor Author

henriquesposito commented Jan 22, 2024

For the time being, shape is not mapped according to variable, it is just set as an aesthetic in autographr() with the '.infer_nshape()' helper function. For this reason, guides also do not work for shape at the moment (see #52).

@henriquesposito
Copy link
Contributor Author

Because we rely on both ggplot2 and ggraph packages in our functions substituting color by fill returns unexpected results (e.g. aesthetics mapped correctly but colors are the same). However, we now consistently map node colors and sizes and this improves the function consistency.

@jhollway jhollway changed the title Rely on fill instead of color for autographr/s/d functions for flexibility and consistency Rely on fill instead of color for graphr/s/t functions for flexibility and consistency Jun 19, 2024
@henriquesposito
Copy link
Contributor Author

@jhollway I am going through the open issues and I am wondering if we should close this issue since it was addressed (as much possible) in the recent changes related to how graphr() works? Thank you.

@jhollway
Copy link
Collaborator

graphr(ison_lawfirm, node_color = "school", node_size = 6) still uses the color geom and not fill.

image

This issue is a bottleneck for themes like 'crisp' that would require e.g. nodes with a separate (white) fill and (black) outline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants