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

prepare method does not warn about unknown parameters #596

Closed
Marius1311 opened this issue Aug 18, 2023 · 5 comments
Closed

prepare method does not warn about unknown parameters #596

Marius1311 opened this issue Aug 18, 2023 · 5 comments
Assignees
Milestone

Comments

@Marius1311
Copy link
Collaborator

I was just wondering why problem.prepare recomputes a PCA, even though I pre-computed one and passed join_attr='X_pca' until I realized that I had a typo in there. The prepare method does not complain about this, which can lead to unexpected behaviour.

@MUCDK
Copy link
Collaborator

MUCDK commented Aug 25, 2023

Hi Marius,

Thanks for opening this issue.

I think we should discuss whether we want to throw an error for unrecognized arguments. We do this in the solve methods, as we can follow a clear logic there, i.e. testing whether an argument is an attribute of the solver class.

In prepare we don't have a logic like this, so we'd have to hard code the arguments we check for.

@michalk8 , do you have an opinion on this?

@giovp
Copy link
Member

giovp commented Aug 25, 2023

we discussed this extensively in the past and I still think it's a pretty bad and unique ux design, wrong arguments should always errors, I understand that we had other priorities but it's something we'll have to figure out sooner than later

@Marius1311
Copy link
Collaborator Author

Thanks for your reply @MUCDK - I fully agree with @giovp, not throwing error for unrecognised arguments can get really confusing.

@selmanozleyen
Copy link
Collaborator

closed with #696

@Marius1311
Copy link
Collaborator Author

amazing, this is great! I think this was very important and will prevent many future mistakes in practical applications.

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

4 participants