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

density with weights #201

Closed
Nosferican opened this issue Nov 26, 2018 · 11 comments
Closed

density with weights #201

Nosferican opened this issue Nov 26, 2018 · 11 comments

Comments

@Nosferican
Copy link

Nosferican commented Nov 26, 2018

Consider

using StatsBase, StatPlots
density(1:3) # Fine
density(1:3, weights = weights([0,500,1])) # No change
density(1:3, weights = [0,500,1]) # No change
density(vcat(1, ones(500), 2)) # Correct
density(1:3, weights([0,500,1])) # Not sure what this is
@mkborregaard
Copy link
Member

Hi, when opening issues, could you please post runnable code (including all usings) and post the plot if it's the plot that is the concern here?
Here's an MWE and a plot based on this:

using StatsBase, StatPlots
plot(
       density(1:3), # Fine
       density(1:3, weights = weights([0,500,1])), # No change
       density(1:3, weights = [0,500,1]), # No change
       density(vcat(1, ones(500), 2)), # Correct
       density(1:3, weights([0,500,1])), # Not sure what this is
legend = false) 

skaermbillede 2018-11-26 kl 08 57 40

So, your concern is that the weights argument to density appears to be ignored by StatPlots?

@Nosferican
Copy link
Author

Will do. That is correct.

@cossio
Copy link

cossio commented May 26, 2019

So, your concern is that the weights argument to density appears to be ignored by StatPlots?

Yes, I'm having the same issue and would definitely like to have a usable weights keyword on density with the same function as the weights option in histogram.

@mkborregaard
Copy link
Member

StatsPlots uses KernelDensity.kde for the density plots. Though I can see a way of hacking support for weights directly here, a more sustainable option might be to request this on KernelDensity? Happy to forward the support here if they add it there.

@Nosferican
Copy link
Author

Just to verify before opening an issue with KernelDensity, the following feature doesn't give you what you would need to adapt the code here? JuliaStats/KernelDensity.jl#26 It was the only issue/PR I found regarding weights in KernelDensity.

@mkborregaard
Copy link
Member

Oh I see it does work, just wasn't documented (where I looked)! Let me have a look.

@Nosferican
Copy link
Author

Perfect! It would still be good to open an issue there to have it documented if that is still the case.

@mkborregaard
Copy link
Member

Does this give what you'd expect @Nosferican ? #232

Skærmbillede 2019-05-27 kl  19 38 13

@Nosferican
Copy link
Author

I think for the first four plots it is what I would expect. 💯I am not sure about the last one. From the documentation, I don't know what would density(::AbstractVector, ::AbstractVector) or density(::AbstractVector, ::Weights) should do. Whatever it is currently doing, is beyond me. 🤷‍♂Maybe that one should give a MethodError. Alternatively,

density(x::AbstractVector, wts::Weights) = density(x, weights = wts)

would make sense and would be what I would argue for as an improvement over current behavior.

@mkborregaard
Copy link
Member

I'd argued for keeping it as is on the PR

@Nosferican
Copy link
Author

Thanks for the PR!

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

No branches or pull requests

3 participants