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

Deprecate the unnecessary function aliasses #180

Closed
richierocks opened this Issue Aug 21, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@richierocks
Copy link
Contributor

richierocks commented Aug 21, 2018

I think that having multiple names for the same function makes things harder, not easier, to understand. For example, get_confidence_interval(), get_ci(), andconf_int() are all identical.

I'd be tempted to keep get_confidence_interval() and deprecate the other two.

Likewise, there is no point in having get_pvalue() and p_value().

Aliases make it harder to read other people's code, since you need to understand the other coder's dialect as well as your own.

@ismayc

This comment has been minimized.

Copy link
Collaborator

ismayc commented Aug 21, 2018

I’m not opposed to this but the flow is not the same as the other verbs in {infer}. I like the get_ variants better as well though get_confidence_interval() is really long, which led to trying to standardize shorter names as aliases.

@echasnovski

This comment has been minimized.

Copy link
Collaborator

echasnovski commented Aug 24, 2018

I really like get_ci() and get_pval() (or existing get_pvalue()) names.

@ismayc

This comment has been minimized.

Copy link
Collaborator

ismayc commented Aug 24, 2018

Given that not everyone uses tab-complete and spelling mistakes for something like get_confidence_interval() are almost surely bound to happen more than get_ci() I think I am leaning towards including it.

Using get_p_value() and get_pvalue() also seems fine to me. I can see having conf_int() and get_ci() as being problematic, but with how similar get_p_value() and get_pvalue() are it is almost like visualize() and visualise() to me.

I think deprecating conf_int() and p_value() makes the most sense here.

@ismayc

This comment has been minimized.

Copy link
Collaborator

ismayc commented Sep 3, 2018

This implementation was added in #189 by @richierocks

@ismayc ismayc closed this Sep 3, 2018

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