Skip to content

MAINT: Remove all nose testing support.#23041

Merged
mattip merged 1 commit intonumpy:mainfrom
charris:remove-nose-testing
Jan 20, 2023
Merged

MAINT: Remove all nose testing support.#23041
mattip merged 1 commit intonumpy:mainfrom
charris:remove-nose-testing

Conversation

@charris
Copy link
Copy Markdown
Member

@charris charris commented Jan 19, 2023

NumPy switched to using pytest in 2018 and nose has been unmaintained for many years. We have kept NumPy's nose support to avoid breaking downstream projects who were might have been using it and not yet switched to pytest or some other testing framework. With the arrival of Python 3.12, unpatched nose will raise an error. It is time to move on.

Decorators removed

  • raises
  • slow
  • setastest
  • skipif
  • knownfailif
  • deprecated
  • parametrize
  • _needs_refcount

These are not to be confused with pytest versions with similar names, e.g., pytest.mark.slow, pytest.mark.skipif, pytest.mark.parametrize.

Functions removed

  • Tester
  • import_nose
  • run_module_suite

@charris charris added 03 - Maintenance 07 - Deprecation 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes labels Jan 19, 2023
@charris charris added this to the 1.25.0 release milestone Jan 19, 2023
reveal_type(np.testing.rundocs("test.py")) # E: None
reveal_type(np.testing.rundocs(Path("test.py"), raise_on_error=True)) # E: None

@np.testing.raises(RuntimeError, RuntimeWarning)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@BvB93 Removing the decorator doesn't seem to bother the tests.

@charris
Copy link
Copy Markdown
Member Author

charris commented Jan 19, 2023

The pyinstaller error looks unrelated. I wonder if the new version 5.7.0 released Dec 4 is the cause.

EDIT: Some of the passing PRs also use the 5.7.0 version.

@mattip
Copy link
Copy Markdown
Member

mattip commented Jan 19, 2023

The error is

Traceback (most recent call last):
  File "numpy/_pyinstaller/pyinstaller-smoke.py", line 14, in <module>
  File "PyInstaller/loader/pyimod02_importers.py", line 499, in exec_module
  File "numpy/__init__.py", line 143, in <module>
  File "PyInstaller/loader/pyimod02_importers.py", line 499, in exec_module
  File "numpy/core/__init__.py", line 100, in <module>
  File "PyInstaller/loader/pyimod02_importers.py", line 499, in exec_module
  File "numpy/core/_add_newdocs.py", line 4970, in <module>
  File "numpy/core/function_base.py", line 529, in add_newdoc
ModuleNotFoundError: No module named 'numpy.core._multiarray_tests'

and seems to be related to this line

add_newdoc('numpy.core._multiarray_tests', 'format_float_OSprintf_g',

@seberg
Copy link
Copy Markdown
Member

seberg commented Jan 19, 2023

It seems raises and run_module_suite are main names available through np.testing. that are removed. We could add a brief:

def __getattr__(self, name):
    # These were removed in 1.25
    if name == "raises" or name == "run_module_suite":
        raise AttributeError(
            "numpy.testing.raises and numpy.random.run_module_suite wrapped "
            "`nose` and has been removed. We recommend switching to `pytest`.")

But, I am happy either way. The typing test looks like the test for the decorator typing itself.

@mattip
Copy link
Copy Markdown
Member

mattip commented Jan 19, 2023

Maybe the error is due to a circular import somehow when using the pyinstaller hook?

@seberg
Copy link
Copy Markdown
Member

seberg commented Jan 19, 2023

There is a nose "excludedimports" in pyinstaller we can delete, but that seems like a tiny cleanup that shouldn't cause a failure. I restarted the test out of curiosity, but no good idea, the hook seems to collect all dynamic libs in numpy, so missing the module seems strange.

@charris
Copy link
Copy Markdown
Member Author

charris commented Jan 19, 2023

Hmm... the noseclasses file imported from _multiarray_tests, maybe we need another import somewhere.

@charris
Copy link
Copy Markdown
Member Author

charris commented Jan 19, 2023

Importing _multiarray_tests in _add_newdocs.py fixes the missing module problem. The question is if that is where we want to import it.

There are a number of profiling errors of the form:

2023-01-19T21:08:46.8102125Z libgcov profiling error:/home/runner/work/numpy/numpy/build/temp.linux-x86_64-3.9/numpy/core/src/common/npy_hashtable.gcda:overwriting an existing profile data with a different timestamp

Which I don't think should be related unless removing nose has exposed a potential profiling problem.

NumPy switched to using pytest in 2018 and nose has been unmaintained
for many years. We have kept NumPy's nose support to avoid breaking
downstream projects who might have been using it and not yet switched to
pytest or some other testing framework. With the arrival of Python 3.12,
unpatched nose will raise an error. It it time to move on.

Decorators removed

- raises
- slow
- setastest
- skipif
- knownfailif
- deprecated
- parametrize
- _needs_refcount

These are not to be confused with pytest versions with similar names,
e.g., pytest.mark.slow, pytest.mark.skipif, pytest.mark.parametrize.

Functions removed

- Tester
- import_nose
- run_module_suite
@charris charris force-pushed the remove-nose-testing branch from acbb5c1 to f75bb0e Compare January 19, 2023 21:42
@charris charris removed the 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes label Jan 19, 2023
@seberg
Copy link
Copy Markdown
Member

seberg commented Jan 19, 2023

Ohhh, the tests are the issue... I suspect you can also add it to:

hiddenimports = ['numpy.core._dtype_ctypes']

in the pyinstaller hook file. Not sure about the other thing, but I doubt it is related.

@charris
Copy link
Copy Markdown
Member Author

charris commented Jan 19, 2023

Ohhh, the tests are the issue... I suspect you can also add it to:

I considered that, made me wonder if pyinstaller was working correctly before. In the absence of complaints I decided to leave it as is, especially as I don't really understand it :)

EDIT1: but maybe "hidden" refers to the underscore prefix? But then I would expect problems with the other underscored modules, but they are all imported up top.

EDIT2: I note that numpy.core._dtype_ctypes is an ordinary Python module, but it imports both ctypes and _ctypes which seem to be standard Python libraries.

@charris
Copy link
Copy Markdown
Member Author

charris commented Jan 19, 2023

We should figure out how to deprecate ModuleDeprecationWarning. I wonder if deprecation warnings work properly in exceptions.

@charris
Copy link
Copy Markdown
Member Author

charris commented Jan 19, 2023

Squashed and release note added.

@charris
Copy link
Copy Markdown
Member Author

charris commented Jan 20, 2023

My guess is that things would work fine if the _pyinstaller directory were moved down below conftest.py, or even if the import were added to _pytesttester.py.

Copy link
Copy Markdown
Member

@seberg seberg left a comment

Choose a reason for hiding this comment

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

Thanks LGTM! Could add formatting to the release note, but doesn't matter.

I am very sure the _multiarray_tests should be in hiddenimport, but I am happy to follow up on that myself. Nothing in NumPy needs/uses it (so there is no way pyinstaller can find it by searching for import statements).
But the tests need it and we want to be able to run them, so we should make sure it gets packaged. Having a stray import in the code works, but seems less clean.

@mattip mattip merged commit eb4ed7f into numpy:main Jan 20, 2023
@mattip
Copy link
Copy Markdown
Member

mattip commented Jan 20, 2023

Thanks @charris

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants