Skip to content

Commit

Permalink
Avoid isinstance(something, int)
Browse files Browse the repository at this point in the history
Testing that arguments where integer by using `isinstance(arg, int)` has
proven itself detrimental on several occasions. Indeed, the test refuses
other types that would be legitimate such as numpy integers. This commit
replaces the remaining occurences of the too narrow test by the solution
that has already been used on multiple places of the code: testing
against `numbers.Integral`. This new way matches everything that looks
enough like an integer, and behave the same on python 2 and python 3 on
the contrary to testing against `int`.
  • Loading branch information
jbarnoud committed Mar 19, 2017
1 parent fdc1e61 commit c6ea5d8
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 16 deletions.
3 changes: 2 additions & 1 deletion package/MDAnalysis/analysis/encore/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
# J. Comput. Chem. 32 (2011), 2319--2327, doi:10.1002/jcc.21787
#
from six.moves import range
import numbers
from multiprocessing.sharedctypes import SynchronizedArray
from multiprocessing import Process, Manager
from joblib import cpu_count
Expand Down Expand Up @@ -67,7 +68,7 @@ def __init__(self, size, metadata=None, loadfile=None):
self.size = size
if loadfile:
self.loadz(loadfile)
elif isinstance(size, int):
elif isinstance(size, numbers.Integral):
self.size = size
self._elements = np.zeros((size + 1) * size / 2, dtype=np.float64)
elif isinstance(size, SynchronizedArray):
Expand Down
5 changes: 3 additions & 2 deletions package/MDAnalysis/auxiliary/XVG.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@

from six.moves import range

import numbers
import os
import numpy as np
from . import base
Expand Down Expand Up @@ -132,7 +133,7 @@ def _select_time(self, key):
if key is None:
# here so that None is a valid value; just return
return
if isinstance(key, int):
if isinstance(key, numbers.Integral):
return self._select_data(key)
else:
raise ValueError('Time selector must be single index')
Expand All @@ -141,7 +142,7 @@ def _select_data(self, key):
if key is None:
# here so that None is a valid value; just return
return
if isinstance(key, int):
if isinstance(key, numbers.Integral):
try:
return self._data[key]
except IndexError:
Expand Down
10 changes: 6 additions & 4 deletions package/MDAnalysis/auxiliary/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,12 @@
from six.moves import range

import os
import numpy as np
import numbers
import math
import warnings

import numpy as np

from ..lib.util import asiterable

from . import _AUXREADERS
Expand Down Expand Up @@ -561,7 +563,7 @@ def __getitem__(self, i):
for auxstep in aux_reader[::10] # analyse every 10th step
run_analysis(auxstep)
"""
if isinstance(i, int):
if isinstance(i, numbers.Integral):
i = self._check_index(i)
return self._go_to_step(i)

Expand All @@ -578,7 +580,7 @@ def __getitem__(self, i):
else self._check_index(i.stop) if i.stop is not None
else self.n_steps)
step = i.step or 1
if not isinstance(step, int) or step < 1:
if not isinstance(step, numbers.Integral) or step < 1:
raise ValueError("Step must be positive integer") # allow -ve?
if start > stop:
raise IndexError("Stop frame is lower than start frame")
Expand All @@ -587,7 +589,7 @@ def __getitem__(self, i):
raise TypeError("Index must be integer, list of integers or slice")

def _check_index(self, i):
if not isinstance(i, (int, np.integer)):
if not isinstance(i, numbers.Integral):
raise TypeError("Step indices must be integers")
if i < 0:
i = i + self.n_steps
Expand Down
6 changes: 3 additions & 3 deletions package/MDAnalysis/coordinates/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ def __getitem__(self, atoms):
correspond to atom indices,
:attr:`MDAnalysis.core.groups.Atom.index` (0-based)
"""
if isinstance(atoms, int):
if isinstance(atoms, numbers.Integral):
return self._pos[atoms]
elif isinstance(atoms, (slice, np.ndarray)):
return self._pos[atoms]
Expand Down Expand Up @@ -1199,7 +1199,7 @@ def apply_limits(frame):
"".format(frame, len(self)))
return frame

if isinstance(frame, int):
if isinstance(frame, numbers.Integral):
frame = apply_limits(frame)
return self._read_frame_with_aux(frame)
elif isinstance(frame, (list, np.ndarray)):
Expand All @@ -1211,7 +1211,7 @@ def apply_limits(frame):

def listiter(frames):
for f in frames:
if not isinstance(f, (int, np.integer)):
if not isinstance(f, numbers.Integral):
raise TypeError("Frames indices must be integers")
yield self._read_frame_with_aux(apply_limits(f))

Expand Down
3 changes: 2 additions & 1 deletion package/MDAnalysis/core/groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
import numpy as np
import functools
import itertools
import numbers
import os
import warnings

Expand Down Expand Up @@ -432,7 +433,7 @@ def __getitem__(self, item):
# because our _ix attribute is a numpy array
# it can be sliced by all of these already,
# so just return ourselves sliced by the item
if isinstance(item, (int, np.int_)):
if isinstance(item, numbers.Integral):
return self.level.singular(self.ix[item], self.universe)
else:
if isinstance(item, list) and item: # check for empty list
Expand Down
9 changes: 5 additions & 4 deletions package/MDAnalysis/core/topologyattrs.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
from collections import defaultdict
import functools
import itertools
import numbers
import numpy as np

from . import flags
Expand Down Expand Up @@ -607,7 +608,7 @@ def __init__(self, values, guessed=False):
def get_residues(self, rg):
resatoms = self.top.tt.residues2atoms_2d(rg._ix)

if isinstance(rg._ix, int):
if isinstance(rg._ix, numbers.Integral):
# for a single residue
masses = self.values[resatoms].sum()
else:
Expand All @@ -621,7 +622,7 @@ def get_residues(self, rg):
def get_segments(self, sg):
segatoms = self.top.tt.segments2atoms_2d(sg._ix)

if isinstance(sg._ix, int):
if isinstance(sg._ix, numbers.Integral):
# for a single segment
masses = self.values[segatoms].sum()
else:
Expand Down Expand Up @@ -933,7 +934,7 @@ class Charges(AtomAttr):
def get_residues(self, rg):
resatoms = self.top.tt.residues2atoms_2d(rg._ix)

if isinstance(rg._ix, int):
if isinstance(rg._ix, numbers.Integral):
charges = self.values[resatoms].sum()
else:
charges = np.empty(len(rg))
Expand All @@ -945,7 +946,7 @@ def get_residues(self, rg):
def get_segments(self, sg):
segatoms = self.top.tt.segments2atoms_2d(sg._ix)

if isinstance(sg._ix, int):
if isinstance(sg._ix, numbers.Integral):
# for a single segment
charges = self.values[segatoms].sum()
else:
Expand Down
3 changes: 2 additions & 1 deletion package/MDAnalysis/core/topologyobjects.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
from __future__ import print_function, absolute_import

from six.moves import zip
import numbers
import numpy as np
import functools

Expand Down Expand Up @@ -751,7 +752,7 @@ def __getitem__(self, item):
Allows indexing via boolean numpy array
"""
# Grab a single Item, similar to Atom/AtomGroup relationship
if isinstance(item, int):
if isinstance(item, numbers.Integral):
outclass = {'bond': Bond,
'angle': Angle,
'dihedral': Dihedral,
Expand Down

0 comments on commit c6ea5d8

Please sign in to comment.