[MRG] Explicitly ignore SparseEfficiencyWarning in DBSCAN

[peay]

What does this implement/fix? Explain your changes.

py is a commonly used library, which has various utility subpackages, one of them being apipkg which implements lazy loading of Python modules. py is marked as a runtime dependency of more than 9K packages on PyPI, so it isn't too easy to avoid.

Importing py itself directly lazily loads certain packages like py.test. Lazy module loading is implemented by adding to sys.modules an apipkg.AliasModule, which implements __getattribute__ as:

        def __getattribute__(self, name):
            try:
                return getattr(getmod(), name)
            except ImportError:
                return None

When py.test is not installed or cannot be imported, this result in sys.modules["py.test"] (and a few others) being AliasModule instances for which module.__warningregistry__ is None (as are all attributes).

This is causing issues with sklearn. Here is an example I've been hitting although I suspect there could be other occurences:

• The DBSCAN implementation uses ignore_warnings to silence warnings.
• ignore_warnings in turns relies on clean_warning_registry.

This makes it impossible to run DBSCAN once py has been imported, because clean_warning_registry is calling clear() on all modules that have a __warningregistry__ attribute:

/opt/venv/python3/lib/python3.6/site-packages/sklearn/cluster/dbscan_.py in dbscan(X, eps, min_samples, metric, metric_params, algorithm, leaf_size, p, sample_weight, n_jobs)
    140 
    141         # set the diagonal to explicit values, as a point is its own neighbor
--> 142         with ignore_warnings():
    143             X.setdiag(X.diagonal())  # XXX: modifies X's internals in-place
    144 

/opt/venv/python3/lib/python3.6/site-packages/sklearn/utils/testing.py in __enter__(self)
    366         self._module.filters = self._filters[:]
    367         self._showwarning = self._module.showwarning
--> 368         clean_warning_registry()
    369         warnings.simplefilter("ignore", self.category)
    370 

/opt/venv/python3/lib/python3.6/site-packages/sklearn/utils/testing.py in clean_warning_registry()
    799             continue
    800         if hasattr(mod, reg):
--> 801             getattr(mod, reg).clear()
    802 
    803 

AttributeError: 'NoneType' object has no attribute 'clear'

This PR adds an additional check that __warningregistry__ is a dictionnary.

Any other comments?

I am not familiar enough with the project to know how we would want this to be fixed. In particular:

• The issue really stems from py's lazily loaded modules, and one may wonder whether sklearn should be worrying about this at all. Here, the issue could also be fixed by making sure py.test can be imported.

• However, __warningregistry__ isn't really documented either, and one could argue that sklearn should be checking whether it is a dictionnary before calling clear on it.

I'll let the maintainers decide whether this small additional check makes sense, in spite of this being a fix for a rather specific problem.

[jnothman]

Thanks for this, though we really shouldn't be using ignore_warnings except in test code. I'd rather change dbscan: if we must ignore warnings we should ignore specific warnings only.

[peay]

Makes sense.

I assume we want to switch to warnings.catch_warnings/warnings.filterwarnings although I am not sure what warning we're actually silencing here. @TomDLT what do you think? Seems like this was added as part of #12105.

[jnothman]

Mea culpa for approving the use of ignore_warnings in library code!

[TomDLT]

My mistake. We should definitely replace ignore_warnings with warnings.catch_warnings/warnings.filterwarnings, ignoring only SparseEfficiencyWarning.

I double checked that it is the only use of ignore_warnings outside tests, examples, and benchmarks.

[peay]

I've commented out the with clause and ran DBSCAN again (with a sparse precomputed metric) to try and figure out what the warning we're trying to silence is, but on scikit-learn==0.20.2, numpy==1.13.1, scipy==1.1.0, I don't get anything. Not sure if the with ignore_warnings() could just be removed or if there are warnings that could appear in other circumstances.

[peay]

Oops, hadn't seen your answer @TomDLT. I will make the change.

[peay]

I was able to reproduce the warning with csr/csc by setting include_self to False in radius_neighbors_graph - it doesn't occur if the input sparse matrix already has all its diagonal entries set since then the operation is efficient. I've pushed the new changes and renamed the PR.

[jnothman]

Please add commits rather than force pushing where possible

[TomDLT]

Thanks @peay !