-
Notifications
You must be signed in to change notification settings - Fork 32
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
relax some checks on eltypes for contrasts and add tests #242
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks pretty straightforward. Get CIs to pass and I'll be happy. This is important enough that I would also immediately tag a patch release.
The x86 failure might be a CSV issue (JuliaData/CSV.jl#913) |
@kleinschmidt patch-bump (0.6.29) and merge? |
In theory I think the rule is to bump the minor version when dropping support for older Julia versions, in case one needs to do another patch release to fix a bug there too. But making a breaking release that requires updating version bounds for all dependencies would be annoying. |
Seems unlikely that we'd need to backport things to make another release that supports unsupported Julia versions, as 1.6 is LTS. |
I'd like to merge the fix in #252 first, then merge that here, make sure CI still passes, adn then we can merge. |
To maintain compat with Julia 1.0, I had to write the tests using CSV to round trip some strings...that should produce
StringN
types for versions where that's possible and do something reasonable otherwise.fixes #236