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

add_group for internal use, enw_assign_group for external use? #214

Closed
seabbs opened this issue Mar 26, 2023 · 2 comments · Fixed by #239
Closed

add_group for internal use, enw_assign_group for external use? #214

seabbs opened this issue Mar 26, 2023 · 2 comments · Fixed by #239
Assignees
Labels
enhancement New feature or request question Further information is requested

Comments

@seabbs
Copy link
Collaborator

seabbs commented Mar 26, 2023

focus the pure technical capabilities in add_group (such as executing on a by clause), but have it assume (because it's internal function) that it's not handed garbage. the enw_assign_group handles all the crap users might throw at it.

Originally posted by @pearsonca in #208 (comment)

@seabbs seabbs added enhancement New feature or request question Further information is requested labels Mar 26, 2023
@seabbs
Copy link
Collaborator Author

seabbs commented Mar 26, 2023

I think we always want to go via assign group (at least for now) but agree we could have assign group be the exposed to the user version and add_group be the less safe internal use version (though for people inspecting the code this may lead them astray (if for example they want to role their own version of the preprocessing function for some reason).

@seabbs
Copy link
Collaborator Author

seabbs commented Apr 24, 2023

Done in #239

@seabbs seabbs closed this as completed Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants