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

Allow positional include args for FilterTransforms in Adapter #249

Open
LarsKue opened this issue Nov 13, 2024 · 1 comment
Open

Allow positional include args for FilterTransforms in Adapter #249

LarsKue opened this issue Nov 13, 2024 · 1 comment
Assignees
Labels
feature New feature or request refactoring Some code shall be redesigned
Milestone

Comments

@LarsKue
Copy link
Contributor

LarsKue commented Nov 13, 2024

The following call will raise a confusing error:

a = bf.Adapter()
a.standardize("x")
>>> TypeError: Adapter.standardize() takes 1 positional argument but 2 were given

However, the intent is very clear: we only want to standardize x. The current way to do this is via an include list which is given as an explicit keyword argument:

a.standardize(include="x")

However, this seems a little bit ugly. We could allow users to pass this as a positional argument as well. So long as none of the keyword-only arguments are defined, we default to turning the positional argument into the include list.

@LarsKue LarsKue added feature New feature or request refactoring Some code shall be redesigned labels Nov 13, 2024
@LarsKue LarsKue added this to the BayesFlow 2.0 milestone Nov 13, 2024
@LarsKue LarsKue self-assigned this Nov 13, 2024
@paul-buerkner
Copy link
Contributor

I agree. makes sense to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request refactoring Some code shall be redesigned
Projects
Status: Future
Development

No branches or pull requests

2 participants