Skip to content

API rename force_all_finite into ensure_all_finite in check_array ? #29262

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

Closed
jeremiedbb opened this issue Jun 14, 2024 · 0 comments · Fixed by #29404
Closed

API rename force_all_finite into ensure_all_finite in check_array ? #29262

jeremiedbb opened this issue Jun 14, 2024 · 0 comments · Fixed by #29404

Comments

@jeremiedbb
Copy link
Member

check_array has several parameters that just enable a check on a property of the array, like ensure_2d, ensure_min_samples, ... They have no effect on the output array: they just have the effect to raise an error or not. They usually have the naming pattern ensure_xxx which I think is intuitive and explicit.

force_all_finite is another example of such behavior but doesn't follow the same naming pattern. I think it should be renamed ensure_all_finite.

  • it would make the current set of params more consistent, intuitive and self explanatory.
  • it would allow to add new params with the naming pattern force_xxx, that have a different behavior e.g. have an effect on the output array, without bringing confusion. This is for instance the case in FEA Add writeable parameter to check_array #29018 that proposes to add force_writeable.

cc @thomasjpfan

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

Successfully merging a pull request may close this issue.

1 participant