-
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
support Arrow on 1.9+ via pkg extension #274
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.
This conceptually looks good to me; some 1.6 tests are failing, however, so I think we either need to add a Requires.jl-based compat or bump up the Julia version; I don't have a real strong preference, though I'm a bit hazy on if 1.9 was declared a new LTS or not. Or is the plan for 1.10 to be an LTS? In either case, this package is currently "slow enough" that even bumping the version up to 1.9 doesn't seem terrible because I don't think there would be a ton of new features or bugfixes that we'd have to try and port back to older versions.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #274 +/- ##
==========================================
+ Coverage 89.67% 89.68% +0.01%
==========================================
Files 10 11 +1
Lines 1821 1823 +2
==========================================
+ Hits 1633 1635 +2
Misses 188 188 ☔ View full report in Codecov by Sentry. |
@quinnj 1.9 isn't the new LTS yet -- LTS can't be declared until the following release is out. But that said, I think bumping compat to 1.9 for a new feature in an otherwise stable package is a good choice and that's what I've done here. We can revisit this decision is there is a loud outcry from 1.6 users. 😄 |
@palday I'm concerned about the Beacon-internal impact of requiring 1.9 here actually. Let's confirm before merging this that that's okay. |
Instead of dropping support for old Julia versions, why not just gate this particular Arrow functionality behind a |
@DilumAluthge The lazy loading of extensions already does that, but I was thinking about adding just such a gate to the tests. |
This reverts commit 13dfdc9.
@quinnj gentle bump 🖤 |
No description provided.