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

"drake_" prefix on the names of all exported functions (intend to not fix) #179

Closed
wlandau-lilly opened this issue Dec 8, 2017 · 2 comments

Comments

@wlandau-lilly
Copy link
Collaborator

wlandau-lilly commented Dec 8, 2017

Supports #147 and ropensci/software-review#156.

The issue of function names often comes up in https://github.com/ropensci/onboarding/issues (for example, @noamross's review of bittrex). @ropensci reviewers frequently recommend a package-specific prefix for all API functions. A name like drake_make() is less risky than make() when it comes to conflicts with other packages, and similar package-specific function names are easily searchable using autocompletion.

Unfortunately, drake violates this convention. A lot. I did not seek out function naming best practices early in development, and I apologize. From my point of view, there are three paths forward.

1. Simply rename all the offending API functions.

This is straightforward, but unwise. Drake has been on CRAN for almost a year, and it caters to a wide user base, so thousands of projects would likely break. Since drake is meant to scale and enhance reproducibility, such a jarring change would be particularly painful for the main intended use cases.

2. Rename and deprecate.

I do not believe this is tractable. Drake has over 101 exported functions, only 12 of which have a drake_ prefix. Deprecating the 89+ offending functions would require thousands more lines of code. The file deprecate.R has all the currently deprecated functions and features, and it is nearly a thousand lines long already.

3. Rename and deprecate only to avoid the most egregious conflicts.

This is what I have been doing in #152, #151, #148, and other batches of commits. The main changes for the upcoming major CRAN update (5.0.0) are documented in NEWS.md:

  • Deprecate and rename functions:
    • analyses() => plan_analyses()
    • as_file() => as_drake_filename()
    • backend() => future::plan()
    • build_graph() => build_drake_graph()
    • check() => check_plan()
    • config() => drake_config()
    • evaluate() => evaluate_plan()
    • example_drake() => drake_example()
    • examples_drake() => drake_examples()
    • expand() => expand_plan()
    • gather() => gather_plan()
    • plan(), workflow(), workplan() => plan_drake()
    • plot_graph() => vis_drake_graph()
    • read_config() => read_drake_config()
    • read_graph() => read_drake_graph()
    • read_plan() => read_drake_plan()
    • render_graph() => render_drake_graph()
    • session() => drake_session()
    • summaries() => plan_summaries()

I think I have dodged the most glaring conflicts, and I am still open to renaming and deprecating functions on a case-by-case basis. @jules32, @benmarwick, and @maelle, I hope this last approach is acceptable for ropensci/software-review#156.

@wlandau-lilly wlandau-lilly changed the title "drake_" prefix on the names of all exported functions "drake_" prefix on the names of all exported functions (intend to not fix) Dec 8, 2017
@maelle
Copy link
Member

maelle commented Dec 8, 2017

Yes it is fine, great job!

@wlandau-lilly
Copy link
Collaborator Author

Thank you, @maelle! I am relieved to hear that!

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

No branches or pull requests

3 participants