PythonPredictions / cobra

A Python package to build predictive linear and logistic regression models focused on performance and interpretation
https://pythonpredictions.github.io/cobra.io
MIT License
30 stars 6 forks source link

Fixing FutureWarnings #156

Closed patrickleonardy closed 1 year ago

patrickleonardy commented 1 year ago

Fixing FutureWarnings

Task: Currently there are a lot of FutureWarinings (generated by pandas) popping up in Cobra that clutter up once notebook. We should change this behavior such that cobra is up to date.

Task Description

This issue will:

pietrodantuono commented 1 year ago

Not a solution, rather a workaround:

In your notebook or script you can suppress the future warnings explicitly as follows:

import warnings
warnings.simplefilter(action='ignore', category=FutureWarning)

Some discussion

Possibility 1

If the warnings stem from the pandas.append action, then the append method should be replaced with pandas.concat.

Possibility 2

Alternatively, one could think of explicitly setting an old version of pandas in the cobra requirements so that pandas won't complain anymore (for the append vs concat example given pandas<1.4.0).

ZlaTanskY commented 1 year ago

Depending on what the warnings are, I disagree with adding a cobra setting to suppress the warnings. If the warnings are on our (cobra) side, we should fix them instead of suppressing them.

sandervh14 commented 1 year ago

To help you decide on this, possibility 1 and 2 are covered with #159 and #160. Since short term fix would be to impose our users to use only pandas<2 (#159), I side with @ZlaTanskY and would say indeed, let's not filter them, imposing the pandas<2 will make sure they never get to see the warnings anyway.

sandervh14 commented 1 year ago

I see in the pull request that you fixed the .loc stuff to avoid the FutureWarnings. With the above two comments, don't ditch the pull request you've made however, it's still relevant to #160! Maybe link the pull request to #160 instead of this issue #156, and if this issue #156 is only about suppressing future warnings, then you can close this one since both me and Jano voted against supressing.

patrickleonardy commented 1 year ago

I did nothing to suppress them I only changed the code that was raising FutureWarnings to an other syntax that does not raise a FutureWarning. Including things that are linked to #160 When you say that I should not ditch it you mean that I should not yet merge it and only merge it with #160 ?