MrTomRod / scoary-2

Calculate assocations between genes and traits
MIT License
19 stars 1 forks source link

Move caching of ConfintStore inside permute_picking. #13

Open njohner opened 4 months ago

njohner commented 4 months ago

Caching an SQL connection on import of scoary works fine when used as a command line tool, but not when scoary is integrated in another python application, as this can lead to the SQL object being used in a different thread than the one it was created in.

This led to issues in the webapplication into which I integrated scoary. I've actually fixed it there (see https://github.com/metagenlab/zDB/pull/101), so I do not need this fix here, but I think it might be useful if anyone else wants to integrate scoary in a python application.

To fix this I simply moved the caching into the permute_picking function, so the connection is renewed every time that function is called. I think that should not have any negative impact on performance.

MrTomRod commented 4 months ago

Thanks for telling me about the bug and suggesting a fix! I did it slightly differently and pushed my fix to the dev branch:

https://github.com/MrTomRod/scoary-2/compare/dev

Do you agree that my fix also solves the issue, and is slightly more elegant?

njohner commented 4 months ago

Hi, Yes looks good. I guess you chose to always connect in the __init__ to check the connection anyway, even when disconnecting right afterwards? Anyway, that works for me thanks!

njohner commented 4 months ago

Oh and I just saw the paper is out, congrats. I'll update the reference in zDB...

MrTomRod commented 4 months ago

Thanks!

I connected in __init__ to create the tables in the database if they don't exist, and then stayed connected just because it's simpler.

Now, it connects in __init__, creates the database, and disconnects. Whenever the function is run, it connects again does it's thing, and disconnects at the end of the function.

I'll wait before creating v0.0.6, ok? Or is it easier for you if I publish the fix?

njohner commented 4 months ago

You can wait, no problem, I'm not in a hurry as I fixed it in zDB for now.