astropy/halotools

Importing halotools.mock_observables changes how numpy floating point errors are handled

Opened this issue · 1 comments

On import, there are a couple of places where halotools runs something like:

np.seterr(divide='ignore', invalid='ignore') # ignore divide by zero in e.g. DD/RR

which will change this globally. In practice, maybe not a huge deal, but this would surprise me as a user (and might catch me out if I was expecting numpy to warn me on divide by 0) and also means there aren't these warnings in the rest of halotools (where you might want them).

@>>> import numpy as np
@>>> np.geterr()
{'divide': 'warn', 'over': 'warn', 'under': 'ignore', 'invalid': 'warn'}
@>>> import halotools.sim_manager
@>>> halotools.sim_manager.rockstar_hlist_reader.np.geterr()
{'divide': 'warn', 'over': 'warn', 'under': 'ignore', 'invalid': 'warn'}
@>>> import halotools.mock_observables
@>>> halotools.sim_manager.rockstar_hlist_reader.np.geterr()
{'divide': 'ignore', 'over': 'warn', 'under': 'ignore', 'invalid': 'ignore'}
@>>> np.geterr()
{'divide': 'ignore', 'over': 'warn', 'under': 'ignore', 'invalid': 'ignore'}

A fix would be to just set this at the start of each function that might need it and then unset it on the way out. I'm not sure how best to do this in python but maybe a with statement that sets it on __enter__ and unsets on __exit__.

hmm, good point about the warning settings. I agree this is not good practice. Both me and @duncandc do this a lot, actually.

It's probably easy to write a context manager that encapsulates these warning settings. It would be better practice to do as Chris says and replace the appearance of these error settings at the top-level module namespace with a context manager operating at the top-level namespace of the user-facing function(s) defined in the module.

@Christopher-Bradshaw - fixing this will be involve pretty low-level, package-maintenance grunt work that is probably not very instructive. So in this case it's probably better if you let me churn through the recursive grep searches to fix this before releasing v0.7. Thanks for pointing out the defect.