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

Inconsistency in parameter names for plot functions #15893

Closed
sebhrusen opened this issue Oct 31, 2023 · 1 comment · Fixed by #15895
Closed

Inconsistency in parameter names for plot functions #15893

sebhrusen opened this issue Oct 31, 2023 · 1 comment · Fixed by #15895
Assignees
Labels
Milestone

Comments

@sebhrusen
Copy link
Contributor

Issue reported by https://support.h2o.ai/a/tickets/106739

Users may use reflection to generate plots, in which case it is much preferable to use consistent naming in plot functions parameters.

User noted that the data param in Py function

def partial_plot(self, data, cols=None, destination_key=None, nbins=20, weight_column=None, ... )

is inconsistent with other plot functions, for example:

def permutation_importance_plot(self, frame, metric="AUTO", n_samples=10000, n_repeats=1, ... )

We need to review plot functions, ensure they all use the same parameter frame and to keep backwards compatibility, we can alias or deprecate the data param.

For consistency between Py and R API, it is also preferrable to apply similar changes in R.

@sebhrusen sebhrusen added the bug label Oct 31, 2023
@sebhrusen sebhrusen added this to the 3.44.0.2 milestone Oct 31, 2023
@sebhrusen sebhrusen self-assigned this Oct 31, 2023
@sebhrusen
Copy link
Contributor Author

after reviewing plot functions, partial_plot seems to be the only plot functions with an inconsistent naming for the data (frame after fix) param

sebhrusen added a commit that referenced this issue Nov 6, 2023
…cy (#15895)

* replaced partial_plot function param 'data' with 'frame', and kept the old param as deprecated

* also renamed param in R for consistency with other R plot functions

* in R, ensure the deprecated args are checked first

* fix dots parameter handling in partialPlot

* fix R doc on partialPlot

* fix R doc on partialPlot

* revert partialPlot handling of dots param using list() as ...names is available only since 4.1.0

* missed a data renamed to newdata

* rename varargs to dots for clarity
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant