EMAworkbench icon indicating copy to clipboard operation
EMAworkbench copied to clipboard

Fix future/deprecation warning

Open EwoutH opened this issue 1 year ago • 9 comments

Fix all of the future/deprecations warnings currently present:

Done (#202, #211):

  • [x] Fix MarkerStyle(None) deprecation in matplotlib by replacing it with MarkerStyle('')
  • [x] Replace deprecated .iteritems() with .items() for dicts
  • [x] Fix Matplotlib deprecation of loc as a positional keyword in legend functions
  • [x] Import launcher from ipyparallel.cluster instead of ipyparallel.apps
  • [x] EMAworkbench/ema_workbench/em_framework/salib_samplers.py:137: DeprecationWarning: salib.sample.saltelli will be removed in SALib 1.5. Please use salib.sample.sobol
  • [x] sklearn/cluster/_agglomerative.py:983: FutureWarning: Attribute affinity was deprecated in version 1.2 and will be removed in 1.4. Use metric instead (#218)
  • [x] DeprecationWarning: zmq.eventloop.ioloop is deprecated in pyzmq 17. pyzmq now works with default tornado and asyncio eventloops. (#334)

No action needed The new default behaviour will be the desired behaviour:

  • [x] EMAworkbench/ema_workbench/analysis/parcoords.py:176: FutureWarning: In a future version, df.iloc[:, i] = newvals will attempt to set the values inplace instead of always setting a new array. To retain the old behavior, use either df[df.columns[i]] = newvals or, if columns are non-unique, df.isetitem(i, newvals)
  • [x] EMAworkbench/EMAworkbench/ema_workbench/analysis/logistic_regression.py:256: FutureWarning: The behavior of DataFrame concatenation with empty or all-NA entries is deprecated. In a future version, this will no longer exclude empty or all-NA columns when determining the result dtypes. To retain the old behavior, exclude the relevant entries before the concat operation. self.peeling_trajectory = pd.concat(

To-do:

  • [ ] EMAworkbench/ema_workbench/analysis/scenario_discovery_util.py:413: UserWarning: FixedFormatter should only be used together with FixedLocator
  • [ ] Python/3.10.8/x64/lib/python3.10/site-packages/tornado/ioloop.py:265: DeprecationWarning: There is no current event loop
  • [ ] Python/3.10.8/x64/lib/python3.10/site-packages/tornado/platform/asyncio.py:299: DeprecationWarning: There is no current event loop

EwoutH avatar Oct 23 '22 22:10 EwoutH

with respect to the open issues

  1. the future behavior is the desired behavior, so in principle, we don't need to do anything.
  2. not entirely sure what is going on here. We probably need to add a set_yticks next to set_yticklabels which should be relatively easy to do. The get_position in line 407 already returns this location, so most likely adding a loc list like the label list and appending the loc from get_position might be sufficient.

https://github.com/quaquel/EMAworkbench/blob/60a45e77c8c720ab9904051124168816f7a6179a/ema_workbench/analysis/scenario_discovery_util.py#L402-L413

However, what concerns me is the difference with the code for handling the columns:

https://github.com/quaquel/EMAworkbench/blob/60a45e77c8c720ab9904051124168816f7a6179a/ema_workbench/analysis/scenario_discovery_util.py#L415-L429

Here a mapping is being used to map labels to locations. I am not sure why this mapping is not used for the rows.

3 and 4. No idea. Where is this warning being triggered in the workbench?

  1. 1.5 is not yet out, so once 1.5 is out we need to deal with this. I have 1.4.5 installed and that does not yet contain the sobol option. 1.4.6 does seem to contain it. So another option is to require 1.4.6 and just move to sobol.

quaquel avatar Oct 24 '22 13:10 quaquel

  1. the future behavior is the desired behavior, so in principle, we don't need to do anything.

Great, moved to the No action needed-list.

3 and 4. No idea. Where is this warning being triggered in the workbench?

3 here and 4 here and 4 here. So all in test/test_em_framework/test_ema_ipyparallel.py::TestLogWatcher::test_extract_level.

EwoutH avatar Oct 25 '22 17:10 EwoutH

So another option is to require 1.4.6 and just move to sobol.

I think this is the most future-proof option. Do we needs some sort of validation that it doesn't (significantly) change outcomes?

EwoutH avatar Oct 27 '22 11:10 EwoutH