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

Fix broken tests #58

Closed
appleparan opened this issue Feb 25, 2020 · 3 comments · Fixed by #67
Closed

Fix broken tests #58

appleparan opened this issue Feb 25, 2020 · 3 comments · Fixed by #67

Comments

@appleparan
Copy link
Contributor

appleparan commented Feb 25, 2020

Some tests are broken due to use of outdated API. Let's fix it.

@rofinn
Copy link
Member

rofinn commented Feb 25, 2020

Can you clarify what you mean? If I recall the only broken tests involve attempts to mutate matrices and tables in cases where that won't work. I'm tempted to drop the current mutating API in favour of supporting separate iterative and static methods. If folks want to mutate the original data they can choose write the results back to the original data if they wish.

@appleparan
Copy link
Contributor Author

appleparan commented Feb 25, 2020

There are some warning messages in CI. [Link]
The warnings are not only complaining not mutating data, but also deprecation. That's the point i want to fix.

By the way, I also want to ask this. Why do you want to drop mutating API? If you don't want to support mutating data, Imputation may require new allocation(!) and this could be huge loss of performance and memory. However, for iterative methods, I agree with your idea. This is not unavoidable.
I think static methods may have current mutating API, but iterative method's impute! should write result back to the original data.

@rofinn
Copy link
Member

rofinn commented Feb 25, 2020

Ahh, okay. Yeah, my plan was to finish removing those deprecations after the next release. I don't want to remove the tests until we're ready to remove support completely. You can always start julia with --depwarn=no.

The problem is that mutability doesn't generalize well for all possible input types. We've had issues with mutability assumptions for specific table and array types (e.g., JuliaData/Tables.jl#116) and we have tests that demonstrate this failure condition.

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

Successfully merging a pull request may close this issue.

2 participants