-
Notifications
You must be signed in to change notification settings - Fork 129
Fix Scan JAX dispatcher #232
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
Conversation
c6a0e5b
to
76d27b2
Compare
Want to simplify a bit the logic with some of the scan helpers now that I know they exist. Can still be reviewed and tested upon! |
76d27b2
to
8b89b2a
Compare
Cleaned up |
86d84a3
to
b0aa0b9
Compare
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.
First round of questions, I am stopping here to not overwhelm you and because I think that some of them might help me understand the rest :)
b0aa0b9
to
d5ba7fe
Compare
@Armavica I addressed some of your suggestions. Feel free to take another stab! |
d5ba7fe
to
7c35a6c
Compare
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.
Ok, I think I understand most of it now. I am just stuck on a few details.
7c35a6c
to
1ba5887
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #232 +/- ##
==========================================
+ Coverage 80.43% 80.46% +0.03%
==========================================
Files 170 170
Lines 45394 45412 +18
Branches 11088 11087 -1
==========================================
+ Hits 36512 36541 +29
+ Misses 6654 6639 -15
- Partials 2228 2232 +4
|
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.
Great, thank you for your patience and your explanations!
Closes #20
Should be working now
NotImplemented: