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

Move/remove geo_type, time_type from as_epi_*(); consider requiring other args by name #632

Open
brookslogan opened this issue Mar 17, 2025 · 0 comments

Comments

@brookslogan
Copy link
Contributor

These are hanging around crowding documentation for these functions.

Consider either

  • Removing geo_type and time_type from the as_*() functions altogether. Though, it seems like time_type overrides could be useful in epi_archives currently, though perhaps as a workaround for other issues; if we do want these overrides, then we'd want to keep these parameters and add functionality back to them.
  • Moving geo_type and time_type to the end of the args list.

Consider also:

  • Moving all but the first argument to as_*() functions behind ... so that they must be called by name. This can help enable better error messages in legacy user code still passing *_type arguments positionally. It also may help code to read better.
    • This option is no longer as clean, as ... is not expected to be empty but now powers rename functionality... probably can still be made to work.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant