-
Notifications
You must be signed in to change notification settings - Fork 109
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
Fix numpy 1.25 deprecation warnings #372
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #372 +/- ##
==========================================
- Coverage 73.36% 73.34% -0.03%
==========================================
Files 22 22
Lines 3315 3316 +1
==========================================
Hits 2432 2432
- Misses 883 884 +1 ☔ View full report in Codecov by Sentry. |
pyoptsparse/pyOpt_solution.py
Outdated
@@ -50,8 +50,8 @@ def __init__(self, optProb, xStar, fStar, lambdaStar, optInform, info): | |||
|
|||
# Now set the f-values | |||
if isinstance(fStar, float) or len(fStar) == 1: | |||
self.objectives[list(self.objectives.keys())[0]].value = float(fStar) | |||
fStar = float(fStar) | |||
self.objectives[list(self.objectives.keys())[0]].value = float(fStar.item()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The .item()
only works when fStar
is a numpy array right? But based on the check above, it may be a float? I'm not entirely sure what the type is (since we don't really have good type coverage)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I guess
- if float, don't do any casting
- if singleton array, use
fStar.item()
withoutfloat()
which is unnecessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahh right, I dont think we need the float check. It should be a numpy array coming into this function (_mapObjtoUser
in _createSolution
will cast it to a numpy array before creating the solution object). I can push some updates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed some changes. Thinking about this a second time though, for multiobjective problems this will be a dict, so it would make sense to just always use a dict instead of an array. That way we dont need any of this logic and there is no difference in the way you access your fStar
. Maybe not convenient, and strictly is a breaking change. Let me know what you think. This would be a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have thought about it too, and I think @marcomangano suggested the same before. But given that almost all of our users do single-objective optimization, I hesitate to unify the variable type for consistency. Happy to discuss further (maybe in an issue?) and yes I would vote for separate PR + probably a minor version bump if we do go for it.
Purpose
Closes #368. In addition, added the
--disallow_deprecations
flag to avoid this happening again. For stable images to pass PR #370 needs to be merged first.Expected time until merged
Once tests pass, and after PR #370
Type of change
Testing
Checklist
flake8
andblack
to make sure the Python code adheres to PEP-8 and is consistently formattedfprettify
or C/C++ code withclang-format
as applicable