OceanParcels / Parcels

Main code for Parcels (Probably A Really Computationally Efficient Lagrangian Simulator)
https://www.oceanparcels.org
MIT License
295 stars 136 forks source link

`is` should not be used for string comparison #542

Closed willirath closed 5 years ago

willirath commented 5 years ago

(cc: @schmidt-christina)

Currently, string comparison is done via identity checking (e.g., interp_method is "linear" in https://github.com/OceanParcels/parcels/blob/master/parcels/field.py#L625 ff.)

This is risky because it relies on implementation details for string interning (see https://stackoverflow.com/questions/15541404/python-string-interning) in Python. To be robust against these subtleties, these comparisons should be done by equality checking (using ==).

willirath commented 5 years ago

A quick demo, how this could be a problem

In [1]: stub = "linea"                                                          

In [2]: (stub + "r") is "linear"                                                
Out[2]: False

In [3]: (stub + "r") == "linear"                                                
Out[3]: True

(And full disclosure: I'm running into this problem when distributing Parcels experiments with Dask.)

erikvansebille commented 5 years ago

Thanks for flagging this, @willirath. While in recent PRs I've mostly been using == instead is, there were still a lot of old uses. You're right that these should be changes, and thanks for doing just that. I've just merged