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

Outsource package-requirement messages to a central place #1109

Closed
schillic opened this issue Feb 12, 2019 · 2 comments · Fixed by #1445
Closed

Outsource package-requirement messages to a central place #1109

schillic opened this issue Feb 12, 2019 · 2 comments · Fixed by #1445
Assignees
Labels
refactoring 🔧 Internal code changes, typically no impact on API simplification 👶 Simplifies code

Comments

@schillic
Copy link
Member

We have quite a few of these guys, sometimes with slightly different text:

@assert isdefined(@__MODULE__, :Polyhedra) "this function requires the package 'Polyhedra'"

I propose to outsource them to a central function to make the code simpler.
Maybe we should even write a macro, e.g., @required (@require is already taken by Requires).

@schillic schillic added simplification 👶 Simplifies code refactoring 🔧 Internal code changes, typically no impact on API labels Feb 12, 2019
@mforets
Copy link
Member

mforets commented Feb 12, 2019

Yes, a macro @requires_polyhedra, or @requires(:Polyhedra) that displays a message similar to "this function needs the package 'Polyhedra'; do 'using Polyhedra'" and then try again would centralize those messages in one place.

@schillic
Copy link
Member Author

schillic commented Feb 12, 2019

@requires_polyhedra

I prefer to have a generic macro, as in your second proposal. Polyhedra is not the only optional package.
Actually, this makes me realize that we generally want to support a list of packages 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring 🔧 Internal code changes, typically no impact on API simplification 👶 Simplifies code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants