-
Notifications
You must be signed in to change notification settings - Fork 52
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
Remove istable for a value of type AbstractMatrix #198
Conversation
I think - as in other places - we should only define a method for a type, and use a default fallback for the value. @quinnj Was there some specific reason why in this case a method for a value was introduced?
I don't remember really |
Looks like there's a test that needs to be updated |
Yes - I will fix it, but first I wanted to check the direction 😄. |
We should fix a bug in TrableTraits.jl and not change anything here. But let us wait till queryverse/TableTraits.jl#21 is resolved. |
I just came across this PR since I noticed that my implementation of |
After @quinnj agrees to the proposal I will update the tests. |
@quinnj - after TableTraits.jl fix the tests pass |
Thanks! |
I just noticed that the PR was made against and merged into master instead of main. That seems incorrect? |
Ah, great catch! I'll merge it over to main |
I think - as in other places - we should only define a method for a type, and use a default fallback for the value. @quinnj Was there some specific reason why in this case a method for a value was introduced?
I think - as in other places - we should only define a method for a type, and use a default fallback for the value.
@quinnj Was there some specific reason why in this case a method for a value was introduced?