Weighted densities support #90
Conversation
| gg_no_wts <- layer_data(ggplot(df, aes(x = x, y = 0)) + stat_density_ridges()) | ||
| d_no_wts <- stats::density(df$x) | ||
|
|
||
| expect_equal(gg_no_wts$density,d_no_wts$y) |
There was a problem hiding this comment.
Please always put a space after a comma (there are a few more cases like that in the following lines).
|
This generally looks good to me. Could you also make an entry in |
|
Thanks for the quick turnaround! Let me know if there's anything else I need to clean up! |
|
Thanks for your contribution! I just recently made a release, so not sure how quickly this will percolate to CRAN. But at least it's in the github version now. |
|
Did this ever get deployed on CRAN? I am not seeing this functionality in the CRAN version, which was uploaded in Jan 2024 (before this commit) https://cran.r-project.org/web/packages/ggridges/index.html Edit: When I try to run the test code: `df <- data.frame(x = rnorm(100),wts = runif(100)) gg_wts <- layer_data(ggplot(df, aes(x = x, y = 0, weight = wts)) + stat_density_ridges())` I get the following: I have not been able to get the weighting to work properly |
|
No, it's not yet on CRAN, sorry. I need to prepare a new release. Haven't gotten around to that yet. If there is a warning message about dropped aesthetics that means ggridges needs to explicitly declare those aesthetics as ones that will be dropped. Beyond that, the warning message should not affect the output. Can you test with the ggridges development version whether weights work as they should? (By testing whether visually the plot changes. Again, the warning can be ignored and needs to be fixed.) |
|
I have just tested the dev version, and the plots do not change whether an
aes(weight=__) is declared vs not, and the warning persists with the dev
version.
…On Wed, Jan 15, 2025 at 4:49 PM Claus Wilke ***@***.***> wrote:
No, it's not yet on CRAN, sorry. I need to prepare a new release. Haven't
gotten around to that yet.
If there is a warning message about dropped aesthetics that means ggridges
needs to explicitly declare those aesthetics as ones that will be dropped.
Beyond that, the warning message should not affect the output.
Can you test with the ggridges development version whether weights work as
they should? (By testing whether visually the plot changes. Again, the
warning can be ignored and needs to be fixed.)
—
Reply to this email directly, view it on GitHub
<#90 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHLRKJOHWXEWF237LED3OOD2K3JX5AVCNFSM6AAAAABVIF43GWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKOJUGAYDSNJTGU>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
That seems strange, because now looking at the code it does correctly declare that |
|
Ah, apologize that part was on my end! The dev version works as expected.
…On Wed, Jan 15, 2025 at 5:00 PM Claus Wilke ***@***.***> wrote:
That seems strange, because now looking at the code it does correctly
declare that weight is a dropped aesthetic and we also added a test for
weighted and unweighted plots so things don't quite add up for me.
—
Reply to this email directly, view it on GitHub
<#90 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHLRKJJ3B3HHVD5PC33TLDT2K3LAZAVCNFSM6AAAAABVIF43GWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKOJUGAZDKNBQGE>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
This is an attempt to resurrect PR59 from @mkoohafkan.
I tried to address the notes on the original PR, narrowing the focus to just adding the weight aesthetic and adding two tests, one for the unweighted density case and one for the weighted case.