AstroViking / tf-kde

Do What The F*ck You Want To Public License
1 stars 2 forks source link

Integration into zfit #2

Open jonas-eschle opened 3 years ago

jonas-eschle commented 3 years ago

I was thinking on what is possibly the best way of moving this inside of zfit so that it can be used there.

Since there are a few, I would suggest that maybe the pure zfit PDFs, we can directly move into zfit. While the TFP ones, we can ask whether this is actually of interest (but this may needs some more work on them, e.g. added tests etc.)

I would propose that either you or me is gonna copy them to zfit, add docs, tests. If you're short on time, I can do that, or parts, and you're very welcome to add things. If you're interested and want to contribute directly - it's your work and it should fit without a problem - please go ahead and add it yourself to zfit and make a PR, that is also very welcome!

Do you have a preference on how to proceed?

Or would you want to promote this to a real package? I think it would just need more polish, unittests, docs, then for sure...

AstroViking commented 3 years ago

Thanks for following up with this! I do not have plans to make this into a real (and maintained) package currently.

There are actually four PDF classes of interest (all located in tf_kde/distribution/kernel_density_estimation_zfit.py).

I started to move the three suitable methods into zfit in the following pull request here: https://github.com/zfit/zfit/pull/285

Since I currently do not have the time to extend this with tests and docs all by myself, your help is very welcome. Also I am not entirely sure about the architecture of zfit and where to put things like utility classes, so please advise me, where I made a suboptimal choice.