-
Notifications
You must be signed in to change notification settings - Fork 337
Fixed #772 Add Ray Support #986
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
Need to add |
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.
@seanlaw
Didn't mean to interrupt your work but just saw one thing and decided to bring your attention to it before I forget.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #986 +/- ##
==========================================
- Coverage 98.93% 97.42% -1.51%
==========================================
Files 84 87 +3
Lines 14293 14893 +600
==========================================
+ Hits 14141 14510 +369
- Misses 152 383 +231 ☔ View full report in Codecov by Sentry. |
@NimaSarajpoor I think this is ready to be reviewed (I know you are super busy so no rush). Please feel free to ask any questions as I'm sure I missed many things since I've stared at it for so long :) |
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.
@seanlaw
I provided a few comments as my first round of review.
@NimaSarajpoor I've made some changes based on your suggestions |
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.
@seanlaw
I am still trying to digest the changes in the test.sh
😅 Btw, I was wondering if you would like to update the README as well by adding something like this:
STUMPY supports `ray` but it is experimental
No, since it is experimental, I would consider adding it in the future IF |
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.
@seanlaw
This should be my last set of comments! That was one big PR with huge amount of effort you put behind it!
@NimaSarajpoor Please feel free to squash and merge if you feel that it is ready. Everything looks good on my part. Thank you for your help! |
@seanlaw |
See #772
Pull Request Checklist
Below is a simple checklist but please do not hesitate to ask for assistance!
black
(i.e.,python -m pip install black
orconda install -c conda-forge black
)flake8
(i.e.,python -m pip install flake8
orconda install -c conda-forge flake8
)pytest-cov
(i.e.,python -m pip install pytest-cov
orconda install -c conda-forge pytest-cov
)black .
in the root stumpy directoryflake8 .
in the root stumpy directory./setup.sh && ./test.sh
in the root stumpy directory