Skip to content

Default 32 bit#14163

Closed
felexkemboi wants to merge 3 commits intoscikit-learn:masterfrom
felexkemboi:default_32_bit
Closed

Default 32 bit#14163
felexkemboi wants to merge 3 commits intoscikit-learn:masterfrom
felexkemboi:default_32_bit

Conversation

@felexkemboi
Copy link
Copy Markdown

Fixes for: #14029.
The doctests would fail if the default numpy.int is 32 bits long.I have a step somewhere for pytest to warn

Copy link
Copy Markdown
Member

@rth rth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR!

I don't think that a warning is a right solution here (cf #14029 (comment)). Let's wait for some feedback by other contributors in that issue.

Also this includes a test.py which shouldn't be part of this PR.

@glemaitre
Copy link
Copy Markdown
Member

I think that @rth as more this in head:

diff --git a/conftest.py b/conftest.py
index f8ced99fc..73326d6d2 100644
--- a/conftest.py
+++ b/conftest.py
@@ -8,10 +8,12 @@
 import platform
 from distutils.version import LooseVersion
 
-from sklearn import set_config
 import pytest
 from _pytest.doctest import DoctestItem
 
+from sklearn import set_config
+from sklearn.utils import _IS_32BIT
+
 PYTEST_MIN_VERSION = '3.3.0'
 
 if LooseVersion(pytest.__version__) < PYTEST_MIN_VERSION:
@@ -51,13 +53,17 @@ def pytest_collection_modifyitems(config, items):
     try:
         import numpy as np
         if LooseVersion(np.__version__) < LooseVersion('1.14'):
+            reason = 'doctests are only run for numpy >= 1.14'
+            skip_doctests = True
+        elif _IS_32BIT:
+            reason = ('doctest are only run when the default numpy int is '
+                      '64 bits.')
             skip_doctests = True
     except ImportError:
         pass
 
     if skip_doctests:
-        skip_marker = pytest.mark.skip(
-            reason='doctests are only run for numpy >= 1.14 and python >= 3')
+        skip_marker = pytest.mark.skip(reason=reason)
 
         for item in items:
             if isinstance(item, DoctestItem):

@rth
Copy link
Copy Markdown
Member

rth commented Jul 2, 2019

Closing as this was resolved in #14167. Thanks for contributing!

@rth rth closed this Jul 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants