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

MNT: Fix warnings in test suite and adds support for numpy 2.0 #623

Merged
merged 23 commits into from
Jun 24, 2024

Conversation

Gui-FernandesBR
Copy link
Member

@Gui-FernandesBR Gui-FernandesBR commented Jun 16, 2024

Pull request type

  • Code maintenance (refactoring, formatting, tests)

Checklist

  • Tests for the changes have been added (if needed)
  • Docs have been reviewed and added / updated
  • Lint (black rocketpy/ tests/) has passed locally
  • All tests (pytest tests -m slow --runslow) have passed locally
  • CHANGELOG.md has been updated (if relevant)

Current behavior

Numpy 2.0 was officially released today, but we cannot run rocketpy with that new version because some methods were deprecated.
Also, running pytest in our repo usually raises a lot of annoying warnings.

New behavior

Fixed, fixed, fixed! Please see the commit history.

Breaking change

  • No

Additional information

@Gui-FernandesBR Gui-FernandesBR self-assigned this Jun 16, 2024
@Gui-FernandesBR Gui-FernandesBR requested a review from a team as a code owner June 16, 2024 22:56
Gui-FernandesBR added a commit that referenced this pull request Jun 16, 2024
@Gui-FernandesBR Gui-FernandesBR force-pushed the mnt/fix-warnings-and-numpy-upgrade branch from a21320c to 554d487 Compare June 16, 2024 22:57
@Gui-FernandesBR Gui-FernandesBR force-pushed the mnt/fix-warnings-and-numpy-upgrade branch from 554d487 to 6a8f4f8 Compare June 16, 2024 22:57
@Gui-FernandesBR Gui-FernandesBR added Bug Something isn't working Tests Regarding Tests labels Jun 16, 2024
@Gui-FernandesBR Gui-FernandesBR added this to the Release v1.X.0 milestone Jun 16, 2024
@Gui-FernandesBR
Copy link
Member Author

This PR was actually partially problematic: https://github.com/RocketPy-Team/RocketPy/pull/552/files
The 3D trajectory is currently being distorted. Fixed too!

@Gui-FernandesBR
Copy link
Member Author

Something is wrong here. The aritmetic priority is mixed, especially when running tests on macOS.
I can't find an easy solution right now.

Copy link

codecov bot commented Jun 23, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 8 lines in your changes missing coverage. Please review.

Project coverage is 73.69%. Comparing base (6af67ac) to head (2a74fa4).
Report is 15 commits behind head on develop.

Files Patch % Lines
rocketpy/environment/environment.py 20.00% 4 Missing ⚠️
rocketpy/plots/flight_plots.py 75.00% 2 Missing ⚠️
rocketpy/_encoders.py 0.00% 1 Missing ⚠️
rocketpy/mathutils/function.py 96.15% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #623      +/-   ##
===========================================
+ Coverage    73.60%   73.69%   +0.09%     
===========================================
  Files           70       70              
  Lines        10292    10304      +12     
===========================================
+ Hits          7575     7594      +19     
+ Misses        2717     2710       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Gui-FernandesBR
Copy link
Member Author

Something is wrong here. The aritmetic priority is mixed, especially when running tests on macOS. I can't find an easy solution right now.

Ok, I have narrowed down the problem, it is a bit clearer now. However, I don't have a solution yet.

The documentation tests are breaking on Python 3.12 with numpy 2.0. Essentially, this is a type promotion problem that was possibly caused by NEP50. This may help to find a solution: https://numpy.org/neps/nep-0050-scalar-promotion.html#alternatives

However, I think the easiest solutions for us would be the removal of the problematic tests.

Below we have an example:

(.venv12) PS RocketPy> pytest rocketpy --doctest-modules
======================================================= test session starts ========================================================
platform win32 -- Python 3.12.4, pytest-8.2.2, pluggy-1.5.0
rootdir: RocketPy
configfile: pyproject.toml
plugins: cov-5.0.0
collected 34 items

rocketpy\environment\environment.py F.F                                                                                       [  8%]
rocketpy\mathutils\function.py FF....                                                                                         [ 26%]
rocketpy\mathutils\vector_matrix.py ................                                                                          [ 73%]
rocketpy\tools.py .........                                                                                                   [100%]

============================================================= FAILURES =============================================================
_________________________ [doctest] rocketpy.environment.environment.Environment.calculate_density_profile _________________________
3084         None
3085
3086         Examples
3087         --------
3088         Creating an Environment object and calculating the density
3089         at Sea Level:
3090
3091         >>> env = Environment()
3092         >>> env.calculate_density_profile()
3093         >>> env.density(0)
Expected:
    1.225000018124288
Got:
    np.float64(1.225000018124288)

RocketPy\rocketpy\environment\environment.py:3093: DocTestFailure
_____________________________ [doctest] rocketpy.environment.environment.Environment.set_gravity_model _____________________________
556         a :class:`Function` object representing the gravity model.
557
558         Examples
559         --------
560         Let's prepare a `Environment` object with a constant gravity
561         acceleration:
562
563         >>> g_0 = 9.80665
564         >>> env_cte_g = Environment(gravity=g_0)
565         >>> env_cte_g.gravity([0, 100, 1000])
Expected:
    [9.80665, 9.80665, 9.80665]
Got:
    [np.float64(9.80665), np.float64(9.80665), np.float64(9.80665)]

RocketPy\rocketpy\environment\environment.py:565: DocTestFailure
_____________________________________ [doctest] rocketpy.mathutils.function.Function.get_value _____________________________________
829         >>> f2.get_value([1, 2, 3], [1, 2, 3])
830         [2, 8, 18]
831         >>> f2.get_value([5], [5])
832         [50]
833
834         Testing with ndarray source (1 dimension):
835         >>> f3 = Function(
836         ...    [(0, 0), (1, 1), (1.5, 2.25), (2, 4), (2.5, 6.25), (3, 9), (4, 16)]
837         ... )
838         >>> f3.get_value(2)
Expected:
    4.0
Got:
    np.float64(4.0)

RocketPy\rocketpy\mathutils\function.py:838: DocTestFailure
_______________________________ [doctest] rocketpy.mathutils.function.Function.is_strictly_bijective _______________________________
2617         -------
2618         result : bool
2619             True if the Function is "strictly" bijective, False otherwise.
2620
2621         Examples
2622         --------
2623         >>> f = Function([[0, 0], [1, 1], [2, 4]])
2624         >>> f.isbijective()
2625         True
2626         >>> f.is_strictly_bijective()
Expected:
    True
Got:
    np.True_

@phmbressan
Copy link
Collaborator

The new promotion rules make it rather difficult to support doctest of both numpy<2.0 and the newest one, since there is a type difference in the __repr__ check: True (for older numpy) and np.True_.

The PR #624 bumps the minimum Python version to 3.9, allowing all the docstest to be executed with numpy>=2.0 allowing for consistency. The code itself (and the tests) still work fine on numpy<2.0.

Therefore, that PR should be merged first.

@Gui-FernandesBR Gui-FernandesBR merged commit 286d315 into develop Jun 24, 2024
11 checks passed
@Gui-FernandesBR Gui-FernandesBR deleted the mnt/fix-warnings-and-numpy-upgrade branch June 24, 2024 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Tests Regarding Tests
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

2 participants