enthought / chaco

Chaco is a Python package for building interactive and custom 2-D plots.
http://docs.enthought.com/chaco/
Other
292 stars 99 forks source link

Tuple/List traits deprecation warning raised in "ImagePlot" #844

Closed rahulporuri closed 1 year ago

rahulporuri commented 1 year ago

Problem Description

The following traits deprecation warning is raised in our application that makes use of ImagePlot. This seems to be because a trait defined as a Union(Tuple, List) is being set a list, which traits seems to be coercing into a tuple instead of accepting the list as is.

C:\Users\rporuri\.edm\envs\<env-name>\lib\site-packages\traits\trait_types.py:4245: DeprecationWarning: In the future, lists will no longer be accepted by the Tuple trait type. Lists should be converted to tuples prior to validation.
  return trait_type_instance.validate(obj, name, value)

https://github.com/enthought/chaco/blob/695f1c0bba3376e8936e9bb47b5b73d8fa7cfea0/chaco/plots/image_plot.py#L85-L87

https://github.com/enthought/chaco/blob/695f1c0bba3376e8936e9bb47b5b73d8fa7cfea0/chaco/plots/image_plot.py#L288

where screen_rect is a list, not a tuple.

https://github.com/enthought/chaco/blob/695f1c0bba3376e8936e9bb47b5b73d8fa7cfea0/chaco/image_utils.py#L15

Reproduction Steps:

N/A

OS, Python version: Windows 11, Python 3.8.12+3

corranwebster commented 1 year ago

I think this is not ideal, but also "fixing" it may lead to issues later.

Currently if you assign a list then you get a tuple, which according to the trait is an acceptable value (and so presumably works). It is surprising, but the fix to that is to have the Tuple trait not accept lists, which is precisely the change being warned about!

In the future, when the Tuple trait rejects lists, the union will cause it to fail over into the List trait, which should then work without issue.

Short of creating a custom trait that behaves now like the future behaviour of Union(Tuple, List, ...) I'm not sure what other fix there might be that doesn't either potentially break existing code, or require future changes.

corranwebster commented 1 year ago

Fixed by #856