Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix various typos #8181

Merged
merged 15 commits into from Jun 24, 2022
Merged

Fix various typos #8181

merged 15 commits into from Jun 24, 2022

Conversation

luzpaz
Copy link
Contributor

@luzpaz luzpaz commented Jun 21, 2022

Found via codespell -q 3 -S CHANGE_LOG -L acount,arry,asscii,ba,breal,documen,hge,inout,larg,nd,nin,splitted,te,tunnell,warmup,withs,wth

@luzpaz
Copy link
Contributor Author

luzpaz commented Jun 21, 2022

Please also review 32c838d closely as it modifies source code.

esc
esc reviewed Jun 21, 2022
View reviewed changes
docs/source/user/faq.rst Outdated Show resolved Hide resolved
esc
esc reviewed Jun 21, 2022
View reviewed changes
numba/np/arrayobj.py Outdated Show resolved Hide resolved
esc
esc reviewed Jun 21, 2022
View reviewed changes
numba/np/arrayobj.py Outdated Show resolved Hide resolved
esc
esc requested changes Jun 21, 2022
View reviewed changes
Copy link
Member

@esc esc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@luzpaz thank you for opening this PR and welcome to Numba! I've given this a look and spotted two more issues. Also, I think this will need to go through an additional CI run before being merged since some of this changes variable names (minor ones, albeit).

@esc esc added 4 - Waiting on author Waiting for author to respond to review and removed 3 - Ready for Review labels Jun 21, 2022
esc
esc previously approved these changes Jun 21, 2022
View reviewed changes
DrTodd13
DrTodd13 previously approved these changes Jun 21, 2022
View reviewed changes
Copy link
Collaborator

@DrTodd13 DrTodd13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve for the code for which I am the owner.

stuartarchibald
stuartarchibald requested changes Jun 22, 2022
View reviewed changes
Copy link
Contributor

@stuartarchibald stuartarchibald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing these typos, much appreciated. There's a few minor things to resolve, and the patch needs conflicts resolving, but otherwise looks good. Thanks again!

numba/core/controlflow.py Outdated Show resolved Hide resolved
numba/core/errors.py Outdated Show resolved Hide resolved
numba/core/controlflow.py Outdated Show resolved Hide resolved
numba/tests/doc_examples/test_structref_usage.py Show resolved Hide resolved
gmarkall
gmarkall previously approved these changes Jun 22, 2022
View reviewed changes
Copy link
Member

@gmarkall gmarkall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve the CUDA-related changes.

@luzpaz luzpaz dismissed stale reviews from gmarkall, DrTodd13, and esc via 435bd3d June 22, 2022 10:06
luzpaz and others added 7 commits June 22, 2022 06:30
@luzpaz
Fix various typos
a449993 
Found via `codespell -q 3 -S CHANGE_LOG -L acount,arry,asscii,ba,breal,documen,hge,inout,larg,nd,nin,splitted,te,tunnell,warmup,withs,wth`
@luzpaz
Fix typo in source comment
9ac8591
@luzpaz
Fix source typos
db81aac
@luzpaz @esc
Update docs/source/user/faq.rst
1fc7f3e 
Co-authored-by: esc <esc@users.noreply.github.com>
@luzpaz @esc
Update numba/np/arrayobj.py
5a17971 
Co-authored-by: esc <esc@users.noreply.github.com>
@luzpaz @esc
Update numba/np/arrayobj.py
7700d25 
Co-authored-by: esc <esc@users.noreply.github.com>
@luzpaz @stuartarchibald
Update numba/core/controlflow.py
2d12da5 
Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
luzpaz and others added 2 commits June 22, 2022 06:30
@luzpaz @stuartarchibald
Update numba/core/errors.py
9963095 
Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
@luzpaz @stuartarchibald
Update numba/core/controlflow.py
b4f99cb 
Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
@gmarkall
Copy link
Member

@luzpaz Many thanks for your updates to this PR. As a quick administrative note, could you please use merges of main instead of rebase/force push please? (I think this is what's happening, but please do let me know if I've misunderstood) - from the reviewing perspective, the use of rebase + force push can cause some confusion with the Github review interface and make it harder to correlate comments with changes and figure out which items are resolved - many thanks in advance!

@luzpaz
Copy link
Contributor Author

luzpaz commented Jun 22, 2022

@gmarkall Sorry about that. First time someone has pointed that out to me.

@gmarkall
Copy link
Member

@gmarkall Sorry about that. First time someone has pointed that out to me.

No problem - thanks for your quick response :-)

esc
esc previously requested changes Jun 23, 2022
View reviewed changes
Copy link
Member

@esc esc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the rebase reverted some recent changes accidentally. I've left a comment to address.

stuartarchibald
stuartarchibald requested changes Jun 23, 2022
View reviewed changes
Copy link
Contributor

@stuartarchibald stuartarchibald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update. I think there was an oversight in the rebase that overwrote a change from #8144, and as there was a rebase I checked the entire diff again and found a couple more incorrect "numpy"'s. I've suggested fixes inline as "suggestions" which should make them easy to just accept if you agree. Many thanks.

docs/source/proposals/jit-classes.rst Outdated Show resolved Hide resolved
numba/np/npdatetime_helpers.py Outdated Show resolved Hide resolved
numba/np/npdatetime_helpers.py Outdated Show resolved Hide resolved
numba/parfors/parfor_lowering.py
Comment on lines +1780 to +1781
occurrences = []
occurrences = [sched_sig[0]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not to be fixed in this PR, but I'm not sure this makes sense, it's direct assignment to the same variable name made in adjacent statements.

numba/tests/test_random.py Outdated Show resolved Hide resolved
numba/tests/test_random.py Outdated Show resolved Hide resolved
luzpaz and others added 6 commits June 23, 2022 18:40
@luzpaz @esc
Update docs/source/proposals/jit-classes.rst
28cdce0 
Co-authored-by: esc <esc@users.noreply.github.com>
@luzpaz @stuartarchibald
Update numba/np/npdatetime_helpers.py
373e9e5 
Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
@luzpaz @stuartarchibald
Update numba/np/npdatetime_helpers.py
6c5903f 
Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
@luzpaz @stuartarchibald
Update numba/tests/test_random.py
91318a3 
Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
@luzpaz @stuartarchibald
Update numba/tests/test_random.py
d59b15f 
Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
@luzpaz
Fix requested by stuartarchibald
78a54af
@esc esc added 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 4 - Waiting on author Waiting for author to respond to review labels Jun 24, 2022
stuartarchibald
stuartarchibald approved these changes Jun 24, 2022
View reviewed changes
Copy link
Contributor

@stuartarchibald stuartarchibald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks for the patch and for doing the additional few fixes.

@stuartarchibald stuartarchibald added 5 - Ready to merge Review and testing done, is ready to merge Effort - short Short size effort needed and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Jun 24, 2022
@stuartarchibald stuartarchibald added this to the Numba 0.56 RC milestone Jun 24, 2022
@stuartarchibald
Copy link
Contributor

@luzpaz Congratulations on your first contribution to Numba!

@sklam sklam dismissed esc’s stale review June 24, 2022 22:17

comments addressed

@sklam sklam merged commit 9491611 into numba:main Jun 24, 2022
@luzpaz luzpaz deleted the typos branch June 25, 2022 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@esc esc esc left review comments

@stuartarchibald stuartarchibald stuartarchibald approved these changes

@gmarkall gmarkall gmarkall left review comments

@DrTodd13 DrTodd13 DrTodd13 left review comments

@sklam sklam Awaiting requested review from sklam sklam is a code owner

Assignees
No one assigned
Labels
5 - Ready to merge Review and testing done, is ready to merge Effort - short Short size effort needed
Projects
None yet
Milestone
Numba 0.56 RC
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@luzpaz @gmarkall @stuartarchibald @esc @DrTodd13 @sklam