search

bakermoran / BayesABTest

A simple to use AB testing framework that lets anyone perform bayesian data analysis
https://pypi.org/project/BayesABTest/
MIT License
20 stars 3 forks source link

Update dependencies, drop support for python <3.8 #34

Closed godric closed 7 months ago

godric commented 2 years ago

What?

Bumping dependencies versions to their latest versions:

Incidentally drops support for python 3.6 & 3.7

Why?

The current version of the dependencies are from over ~1.5 years ago (around Jan 2021) Working with this module in combination with other up-to-date modules is getting increasingly difficult as the last year has seen some significant changes in the Python ecosystem.

As far as python versions are concerned, numpy has dropped support for:

Python 3.8 itself was released in Oct. 2019 and the current industry standard seems to be 3.9 (eg: Debian stable currently ships with 3.9). With 3.10 around the corner, it seems reasonable to drop support for 3.6, and 3.7 as numpy did.

WDYT?

godric commented 2 years ago

On a side note, great module 💯 . Coming from a home-brewed pymc3 implementation, this has been a pleasure to work with.

godric commented 2 years ago

☝️ The latest seaborn version is 0.11.2 (this module uses 0.10.1), however a breaking change introduced in 0.11.0 breaks the colouring in _plot_positive_lift() ie: The part responsible for the red shading

Screenshot 2022-05-17 at 13 47 26

I took a stab at it, but it doesn't seem like this updated implementation of kdeplot is going to allow for this extra shading in any simple way. I've decided to leave it out of this PR which is mostly concerned about numpy/pandas anyway.

bakermoran commented 1 year ago

@godric Hey, so sorry I missed this for so long. I need to update my notifications or something. Looks good to me!

godric commented 1 year ago

@bakermoran, no worries happy to hear from you

So since my original PR, numpy (in 1.24) dropped np.float which seaborn uses. I've fixed my fork (specified a working range of version) so it would pass the tests. It should be ready to merge now.

I was wondering if you would be willing to let go of that red shading? If so I would be happy to create a new PR which would update all dependencies to their latest versions, and add more recent python3 version to the test matrix.

godric commented 1 year ago

I was wondering if you would be willing to let go of that red shading? If so I would be happy to create a new PR which would update all dependencies to their latest versions, and add more recent python3 version to the test matrix.

Actually I did just that in https://github.com/godric/BayesABTest/tree/reupdated-deps

bakermoran commented 7 months ago

@godric Sorry I've turned on email notifications for this now to make sure I don't miss anything.

I've merged this PR as is. Open to turning that fork into a PR here. Can you help me understand the problem a bit? Is the goal just to upgrade seaborn, which removed functionality we use here? Or is there a breaking change in another package?

bakermoran commented 7 months ago

@godric This should be available in v1.0.9 now. Thanks for the help here!