Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove references to deprecated NumPy type aliases. #22965

Merged
merged 1 commit into from Dec 23, 2022

Conversation

vrabaud
Copy link
Contributor

@vrabaud vrabaud commented Dec 15, 2022

This change replaces references to a number of deprecated NumPy type aliases (np.bool, np.int, np.float, np.complex, np.object, np.str) with their recommended replacement (bool, int, float, complex, object, str).

Those types were deprecated in 1.20 and are removed in 1.24, cf numpy/numpy#22607.

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work

@vrabaud
Copy link
Contributor Author

vrabaud commented Dec 15, 2022

Do we want to backport to 3.4 ? I am not sure on the Numpy version that is supported there.

@asmorkalov
Copy link
Contributor

Yes, backport to 3.4 is useful too.

@vrabaud vrabaud changed the base branch from 4.x to 3.4 December 20, 2022 09:00
@vrabaud vrabaud changed the base branch from 3.4 to 4.x December 20, 2022 09:00
Copy link
Contributor

@VadimLevin VadimLevin left a comment

Choose a reason for hiding this comment

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

It makes sense to keep these types in test_misc.py, but protect them with version check. Otherwise inaccurate refactoring of conversion code might break something for older versions of NumPy, because coverage is reduced.

@vrabaud
Copy link
Contributor Author

vrabaud commented Dec 20, 2022

@VadimLevin , how would you protect them with version check knowing that np.float (for example) is not defined anymore ?

@VadimLevin
Copy link
Contributor

@VadimLevin , how would you protect them with version check knowing that np.float (for example) is not defined anymore ?

Just several thoughts:

  • numpy is a module and as consequence is a Python object.
    Every Python object can be introspected with built-in hasattr/getattr functions.

  • When all deprecated type aliases will be removed (e.g. numpy.float) you will get AttributeError only when you access these attribute.

  • This cases might be moved to separate test functions that can be skipped using NumPy version checks or attribute checks in unittest.skipIf

@vrabaud
Copy link
Contributor Author

vrabaud commented Dec 20, 2022

I believe the build error is unrelated: https://pullrequest.opencv.org/buildbot/builders/precommit_windows64/builds/100389/steps/test_python2/logs/stdio

@hawkinsp
Copy link

Drive-by comment: you can probably just write bool, float, int etc., since the NumPy types (e.g., np.int etc.) have been aliases for the Python builtins for a very long time. I verified this at least as far back as NumPy 1.15 which is very old at this point.

@vrabaud
Copy link
Contributor Author

vrabaud commented Dec 20, 2022

@hawkinsp , do you have a link for that please ? It would make this PR simpler. Thx

@hawkinsp
Copy link

hawkinsp commented Dec 20, 2022

The release notes of NumPy 1.20 note these type aliases have existed for a "long time". I don't know how far back they go!

https://numpy.org/doc/stable/release/1.20.0-notes.html#using-the-aliases-of-builtin-types-like-np-int-is-deprecated

I personally verified the types are aliases as far back as NumPy 1.15.0 which was released in 2018:

Python 3.7.12 (default, Jan 31 2022, 14:15:00)
Type 'copyright', 'credits' or 'license' for more information
IPython 7.31.1 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import numpy as np

In [2]: np.float is float
Out[2]: True

In [3]: np.__version__
Out[3]: '1.15.0'

I don't have an older copy of Python to try older NumPy versions, but I think it's safe to assume they existed before that.

So if I were you I think I'd be quite comfortable just writing float instead of np.float since they have been aliases into antiquity, and you probably shouldn't support NumPy versions older than that anyway (see https://numpy.org/neps/nep-0029-deprecation_policy.html )

@vrabaud
Copy link
Contributor Author

vrabaud commented Dec 20, 2022

Thx a bunch @hawkinsp ! @VadimLevin , can I revert to what I did initially ? Just np.* -> * ? Thx

@asmorkalov
Copy link
Contributor

I checked OpenCV-Python package. The minimum supported Numpy version is 1.13.3. Python 3.6 + Numpy 1.13.3 handles array names without "np." suffix correctly:

Python 3.6.9 (default, Nov 25 2022, 14:10:45) 
[GCC 8.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import numpy as np
>>> q = np.array([1], dtype=float)
>>> print(q)
[ 1.]
>>> np.__version__
'1.13.3'
>>> 

So you can just remove package name from types.

@VadimLevin
Copy link
Contributor

Thanks @hawkinsp and @asmorkalov. @vrabaud Yep, sounds good. So just remove duplicates from test cases e.g.4.3, np.float(2.5), np.float32(4.25) -> 4.3, np.float(4.25)

modules/python/test/test_misc.py Show resolved Hide resolved
modules/python/test/test_misc.py Outdated Show resolved Hide resolved
modules/python/test/test_misc.py Show resolved Hide resolved
modules/python/test/test_misc.py Show resolved Hide resolved
modules/python/test/test_misc.py Outdated Show resolved Hide resolved
@asmorkalov
Copy link
Contributor

There are more failed tests, if per-module tests are enabled. Command example:

python3.8 ../opencv-master/modules/python/test/test.py -v --repo=/home/alexander/Projects/OpenCV/opencv-master
test_aruco_detector (test_objdetect_aruco.aruco_objdetect_test) ... ERROR
test_aruco_detector_refine (test_objdetect_aruco.aruco_objdetect_test) ... ERROR
test_getDistanceToId (test_objdetect_aruco.aruco_objdetect_test) ... ERROR
test_identify (test_objdetect_aruco.aruco_objdetect_test) ... ERROR
test_idsAccessibility (test_objdetect_aruco.aruco_objdetect_test) ... ERROR
test_peopledetect (test_peopledetect.peopledetect_test) ... ok
test_detect (test_qrcode_detect.qrcode_detector_test) ... ok
test_detect_and_decode (test_qrcode_detect.qrcode_detector_test) ... ok
test_detect_and_decode_multi (test_qrcode_detect.qrcode_detector_test) ... ok
test_detect_multi (test_qrcode_detect.qrcode_detector_test) ... ok
test_simple (test_stitching.stitching_compose_panorama_args) ... ok
test_simple (test_stitching.stitching_compose_panorama_test_no_args) ... ok
test_simple (test_stitching.stitching_detail_test) ... ok
test_simple (test_stitching.stitching_matches_info_test) ... ok
test_simple (test_stitching.stitching_range_matcher_test) ... FAIL
test_simple (test_stitching.stitching_seam_finder_graph_cuts) ... FAIL
test_simple (test_stitching.stitching_test) ... ok
test_lk_homography (test_lk_homography.lk_homography_test) ... ok
test_lk_track (test_lk_track.lk_track_test) ... ok
test_createTracker (test_tracking.tracking_test) ... ok
test_add (test_gapi_core.gapi_core_test) ... ok
test_add_uint8 (test_gapi_core.gapi_core_test) ... ok
test_kmeans (test_gapi_core.gapi_core_test) ... ok
test_kmeans_2d (test_gapi_core.gapi_core_test) ... ERROR
test_kmeans_3d (test_gapi_core.gapi_core_test) ... ERROR

@asmorkalov
Copy link
Contributor

Also strange failure in core things:

======================================================================
FAIL: test_native_method_can_be_patched (test_misc.CanUsePurePythonModuleFunction)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/projects/Projects/OpenCV/opencv-master/modules/python/test/test_misc.py", line 759, in test_native_method_can_be_patched
    self.assertTrue(isinstance(res, Sequence),
AssertionError: False is not true : Overwritten method should return sequence. Got: 10 of type <class 'int'>

@asmorkalov
Copy link
Contributor

@VadimLevin could you take a look on it?

@asmorkalov
Copy link
Contributor

@vrabaud I added couple of more fixes to your PR.

@vrabaud
Copy link
Contributor Author

vrabaud commented Dec 21, 2022

@asmorkalov , thx !

@asmorkalov
Copy link
Contributor

@vrabaud Please ignore my previous message about errors. I fould yet another OpenCV-Python instance.

@asmorkalov
Copy link
Contributor

@vrabaud @VadimLevin I fixed double/float issue and squashed commits. Please take a look ASAP.

modules/python/test/test_misc.py Outdated Show resolved Hide resolved
modules/python/test/test_misc.py Show resolved Hide resolved
This change replaces references to a number of deprecated NumPy
type aliases (np.bool, np.int, np.float, np.complex, np.object,
np.str) with their recommended replacement (bool, int, float,
complex, object, str).

Those types were deprecated in 1.20 and are removed in 1.24,
cf numpy/numpy#22607.
@asmorkalov asmorkalov merged commit 9012e6d into opencv:4.x Dec 23, 2022
@vrabaud vrabaud deleted the numpy_fix branch December 28, 2022 12:16
@alalek alalek mentioned this pull request Jan 8, 2023
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.

None yet

6 participants