DOC: Typos found by codespell #19911
Conversation
DimitriPapadopoulos
force-pushed
the
codespell
branch
2 times, most recently
from
d9a0dfc to
3c11af3
Compare
bashtage
left a comment
bashtage
left a comment
There was a problem hiding this comment.
One definite wrong fix, one small quibble about whether keyserver is a technical word/
| for (; count > 0; count -= vstep, data0 += vstep, data1 += vstep) { | ||
| npyv_@sfx@ a = npyv_load_tillz_@sfx@(data0, count); | ||
| npyv_@sfx@ b = npyv_load_tillz_@sfx@(data1, count); | ||
| vaccum = npyv_muladd_@sfx@(a, b, vaccum); |
There was a problem hiding this comment.
This fix is wrong. The work is not vacuum. It is essentially v_accum.
There was a problem hiding this comment.
Ah, sorry, I thought it was some sort of joke because of the initialization to 0's, or something like that. Will revert it.
There was a problem hiding this comment.
Changed to v_accum.
| const int is_aligned = EINSUM_IS_ALIGNED(data); | ||
| const int vstep = npyv_nlanes_@sfx@; | ||
| npyv_@sfx@ vaccum = npyv_zero_@sfx@(); | ||
| npyv_@sfx@ vacuum = npyv_zero_@sfx@(); |
There was a problem hiding this comment.
Please restore all vaccum to vaccum, or possibly change to v_accum.
There was a problem hiding this comment.
Yes, perhaps v_accum would be better.
DimitriPapadopoulos
force-pushed
the
codespell
branch
from
3c11af3 to
ee00d75
Compare
melissawm
left a comment
melissawm
left a comment
There was a problem hiding this comment.
Looks mostly ok to me, I'm just not sure about the variable names.
| { | ||
| /* Allow calling the function multiple times. */ | ||
| static npy_bool initalized = NPY_FALSE; | ||
| static npy_bool initialized = NPY_FALSE; |
There was a problem hiding this comment.
I'm always weary of correcting variable names even if there is a typo, just my 2 cents here.
There was a problem hiding this comment.
I do understand that. On the other hand, code is for humans, typos don't help, and tests are supposed to catch errors. I really don't know.
There was a problem hiding this comment.
It's just a local variable of the very small get_sfloat_dtype() function, it cannot even shadow other variables. Also, this is C code, and it compiles without a problem.
I feel this modification is very safe.
numpy/ma/tests/test_mrecords.py
Outdated
| # `delimitor` is misspelled | ||
| mrectxt = fromtextfile(path, delimitor=',', varnames='ABCDEFG') |
There was a problem hiding this comment.
Should we open an issue to fix this? Not sure if this gets exposed to the users at all.
There was a problem hiding this comment.
Yes, I was planning another PR for that, if needed.
There was a problem hiding this comment.
Indeed, the numpy.ma.mrecords submodule is not exported, only numpy.ma.core and numpy.ma.extras are:
https://github.com/numpy/numpy/blob/c2ad604/numpy/ma/__init__.py#L48
Also missing from the Masked Arrays documentation.
That will be easy to fix here.
There was a problem hiding this comment.
I'm not sure what you mean by "not exported". numpy.ma.mrecords is "public API" I think (although I suspect an extremely unused part of it).
There was a problem hiding this comment.
Ooopst :/! I missed that this ended up being changed. Could you make a PR to revert this?
EDIT: I am not 100% sure if we cannot get away with changing it, but we can discuss on the reverse PR.
There was a problem hiding this comment.
It's not in numpy.ma.__all__:
https://github.com/numpy/numpy/blob/c2ad604/numpy/ma/__init__.py#L48
It's not in the documentation, either.
This doesn't mean it's not part of the public API, though.
I can submit another PR, where I would restore the delimitor parameter, undocumented, alongside the new delimiter parameter. When both are defined, I would give delimiter priority - or maybe delimitor for total backwards compatibility?
DimitriPapadopoulos
force-pushed
the
codespell
branch
from
8679076 to
8396026
Compare
|
For the future reference, fixing typos in NEPs is OK IMHO. |
|
Thanks @DimitriPapadopoulos and everyone going through the changes! LGTM, and I am happy to change those variable typos. |
6 participants
Fix typos found by codespell.