Fixed #930 Improve ignore_trivial Docstring and Warnings#1110
Fixed #930 Improve ignore_trivial Docstring and Warnings#1110seanlaw merged 5 commits intostumpy-dev:mainfrom
ignore_trivial Docstring and Warnings#1110Conversation
|
Review these changes at https://app.gitnotebooks.com/stumpy-dev/stumpy/pull/1110 |
|
@NimaSarajpoor Would you mind providing a review at your earliest convenience? |
NimaSarajpoor
left a comment
There was a problem hiding this comment.
@seanlaw
I've added a few comments for your consideration.
stumpy/core.py
Outdated
There was a problem hiding this comment.
Should we remove the last sentence?
Default is
Nonewhich corresponds to a self-join.
This is true if we look at it from stump's perspective. From this function's standpoint, I feel that we should not provide such statement. Are we allowing the function to accept None for T_B? If yes, then we can modify this function to do what the new function core.check_self_join does since we can identify if we are dealing with self-join or not. (see the response I've provided for your comment in the new function)
|
@NimaSarajpoor I've made a few small changes based on your review. I think it's ready to be merged unless anything stands out to you? |
|
Nothing stands out to me. All good! 👍 |
|
Thanks @NimaSarajpoor! |
Fixed #930 and #929
Pull Request Checklist
Below is a simple checklist but please do not hesitate to ask for assistance!
black(i.e.,python -m pip install blackorconda install -c conda-forge black)flake8(i.e.,python -m pip install flake8orconda install -c conda-forge flake8)pytest-cov(i.e.,python -m pip install pytest-covorconda install -c conda-forge pytest-cov)black --exclude=".*\.ipynb" --extend-exclude=".venv" --diff ./in the root stumpy directoryflake8 --extend-exclude=.venv ./in the root stumpy directory./setup.sh dev && ./test.shin the root stumpy directory