-
-
Notifications
You must be signed in to change notification settings - Fork 365
Cython Workaround #2911
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
Cython Workaround #2911
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2911 +/- ##
=======================================
Coverage 99.67% 99.67%
=======================================
Files 115 115
Lines 17308 17308
Branches 3109 3109
=======================================
Hits 17252 17252
Misses 38 38
Partials 18 18
|
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 good, I think actually doing the CI work can be done later (just repurpose the already made issue).
I'd be OK with shipping this as a fix for the current problem, but I'm also keen to get some regression tests into our CI so we can tell that it's effective and notice if it breaks later. Maybe @jakkdl can help out? |
I'll take a look at it, seems quite doable. But agree on merging this and getting CI setup in a later PR. |
Ok, with two approvals I am going to merge this. |
This pull request partially completes changes that were noted in #2908.
Either added on to this pull request or in another pull request, we still need to make sure to test that cython will continue working and not find these sorts of issues after they are already a problem.
Excerpt from original comment in #2908: