Repository: numpy/numpy
Pull request: MAINT: dir(numpy) returns duplicate "testing"
Fixes #15383 

Modified `__dir__()` to remove duplicate "Tester/Testing" attribute. Also added a test to verify this.

Problem statement:
dir(numpy) returns duplicate "testing"
<!-- Please describe the issue in detail here, and fill in the fields below -->

### Reproducing code example:

<!-- A short code example that reproduces the problem/missing feature. It should be
self-contained, i.e., possible to run as-is via 'python myproblem.py' -->

```python
import numpy as np
>>> np.__version__
'1.18.1'
>>> len(dir(np))
620
>>> np.testing
<module 'numpy.testing' from 'C:\\Python\\Python38\\lib\\site-packages\\numpy\\testing\\__init__.py'>
>>> len(dir(np))
621
>>> [i for i in dir(np) if i == "testing"]
['testing', 'testing']
```
### Error:
"testing" appears twice in dir(np)


### Numpy/Python version information:

<!-- Output from 'import sys, numpy; print(numpy.__version__, sys.version)' -->
Python 3.8.0 (tags/v3.8.0:fa919fd, Oct 14 2019, 19:37:50) [MSC v.1916 64 bit (AMD64)] on win32
>>> np.__version__
'1.18.1'

Hints from issue discussion:
See a fix at https://stackoverflow.com/questions/59851684/python-duplicates-in-dir-is-it-a-bug
@pyscripter want to open a PR wih that fix (and best a test)?

Commit message: Fixes #15383

Patch to review:
diff --git a/numpy/__init__.py b/numpy/__init__.py
index 73e979a1841d..8636546876b8 100644
--- a/numpy/__init__.py
+++ b/numpy/__init__.py
@@ -218,7 +218,7 @@ def __getattr__(attr):
                                      "{!r}".format(__name__, attr))
 
         def __dir__():
-            return list(globals().keys()) + ['Tester', 'testing']
+            return list(globals().keys() | {'Tester', 'testing'})
 
     else:
         # We don't actually use this ourselves anymore, but I'm not 100% sure that
diff --git a/numpy/tests/test_public_api.py b/numpy/tests/test_public_api.py
index b4aa7ec3da84..2eded9235e75 100644
--- a/numpy/tests/test_public_api.py
+++ b/numpy/tests/test_public_api.py
@@ -94,6 +94,19 @@ def test_import_lazy_import(name):
     assert name in dir(np)
 
 
+@pytest.mark.parametrize('name', ['testing', 'Tester'])
+def test_dir_testing(name):
+    """
+    Test "dir" is not returning duplicate attribute for testing/Tester
+    """
+    exe = (sys.executable, '-c', "import numpy; numpy." + name)
+    result = subprocess.check_output(exe)
+    assert not result
+
+    # Make sure there is only one "testing/tester" in dir
+    attr_count = dir(np).count(name)
+    assert attr_count==1
+
 def test_numpy_linalg():
     bad_results = check_dir(np.linalg)
     assert bad_results == {}


Code review comments:
diff_hunk: @@ -94,6 +94,19 @@ def test_import_lazy_import(name):
     assert name in dir(np)
 
 
+@pytest.mark.parametrize('name', ['testing', 'Tester'])
+def test_dir_testing(name):
+    """
+    Test "dir" is not returning duplicate attribute for testing/Tester
+    """
+    exe = (sys.executable, '-c', "import numpy; numpy." + name)
path: numpy/tests/test_public_api.py
text: @user1:
What is this for?

@author:
This is to verify that explicitly calling numpy.testing is not adding a second copy of 'testing' to the result of dir.

@user1:
But this is started in a separate process, so can have no effect on the rest of the test.

@user2:
At this point in the test we probably already did anything bad (e.g. testing is definitely imported), wouldn't it be enough to just test that `len(dir(np)) == len(set(dir(np)))`, with a comment of why?

@author:
@user3-wieser @user2 I have modified the test to assert len(dir(np)) == len(set(dir(np)))

@user2:
Thanks, seems fair enough to me. If you are still enthusiastic: We try to keep the lines below 80 characters long (so the comment is a bit too long). Might as well rebase/squash the commits (but we can also do both of that).

@author:
@user2  Thanks for the review. I have modified the comments to be inline with the standards.