Skip to content

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

Merged
merged 32 commits into from
Jul 6, 2024
Merged

Fixed #772 Add Ray Support #986

merged 32 commits into from
Jul 6, 2024

Conversation

seanlaw
Copy link
Contributor

@seanlaw seanlaw commented Jun 9, 2024

See #772

Pull Request Checklist

Below is a simple checklist but please do not hesitate to ask for assistance!

  • Fork, clone, and checkout the newest version of the code
  • Create a new branch
  • Make necessary code changes
  • Install black (i.e., python -m pip install black or conda install -c conda-forge black)
  • Install flake8 (i.e., python -m pip install flake8 or conda install -c conda-forge flake8)
  • Install pytest-cov (i.e., python -m pip install pytest-cov or conda install -c conda-forge pytest-cov)
  • Run black . in the root stumpy directory
  • Run flake8 . in the root stumpy directory
  • Run ./setup.sh && ./test.sh in the root stumpy directory
  • Reference a Github issue (and create one if one doesn't already exist)

@seanlaw
Copy link
Contributor Author

seanlaw commented Jun 9, 2024

Need to add ray example to docstrings

Copy link
Collaborator

@NimaSarajpoor NimaSarajpoor left a 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.

Copy link

codecov bot commented Jun 10, 2024

Codecov Report

Attention: Patch coverage is 20.21277% with 225 lines in your changes missing coverage. Please review.

Project coverage is 97.42%. Comparing base (f8f245c) to head (8af81d0).
Report is 51 commits behind head on main.

Files Patch % Lines
tests/test_ray.py 38.18% 68 Missing ⚠️
stumpy/stumped.py 6.52% 43 Missing ⚠️
stumpy/aamped.py 2.77% 35 Missing ⚠️
stumpy/mstumped.py 10.52% 34 Missing ⚠️
stumpy/maamped.py 11.76% 30 Missing ⚠️
ray_python_version.py 0.00% 10 Missing ⚠️
stumpy/core.py 37.50% 5 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

@seanlaw
Copy link
Contributor Author

seanlaw commented Jun 12, 2024

@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 :)

Copy link
Collaborator

@NimaSarajpoor NimaSarajpoor left a 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.

@seanlaw
Copy link
Contributor Author

seanlaw commented Jun 30, 2024

@NimaSarajpoor I've made some changes based on your suggestions

Copy link
Collaborator

@NimaSarajpoor NimaSarajpoor left a 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

@seanlaw
Copy link
Contributor Author

seanlaw commented Jul 2, 2024

Btw, I was wondering if you would like to update the README as well

No, since it is experimental, I would consider adding it in the future IF ray (and numpy, scipy, and numba too) has no issues after 2-3 version updates.

Copy link
Collaborator

@NimaSarajpoor NimaSarajpoor left a 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!

@seanlaw
Copy link
Contributor Author

seanlaw commented Jul 6, 2024

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!

@NimaSarajpoor NimaSarajpoor merged commit fb6b8b8 into stumpy-dev:main Jul 6, 2024
33 checks passed
@NimaSarajpoor
Copy link
Collaborator

@seanlaw
Done! (Thanks for the challenge!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants