-
-
Notifications
You must be signed in to change notification settings - Fork 19.3k
STYLE: Extending codespell to pandas/tests #40320
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
STYLE: Extending codespell to pandas/tests #40320
Conversation
|
Nice, thanks @01-vyom for doing this! This changes lots of files though, could you please split it up into smaller PRs? Maybe 3 PRs doing about 20 files each? |
|
Ok, will do that |
| - id: codespell | ||
| types_or: [python, rst, markdown] | ||
| files: ^(pandas|doc)/ | ||
| exclude: ^pandas/tests/ |
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.
if we exclude this, then the CI check will fail - could you keep it in, we'll update these 22 files, we'll then have another PR to update another ~20 files, and then a final PR which updates the rest and removes exclude from this file?
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.
OK got it.
pandas/tests/io/json/test_pandas.py
Outdated
| ) | ||
| unser = read_json(s.to_json(orient="records"), orient="records", typ="series") | ||
| tm.assert_numpy_array_equal(s.values, unser.values) | ||
| user = read_json(s.to_json(orient="records"), orient="records", typ="series") |
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.
I guess this was meant to stand for unserialized, perhaps rename it to that?
| # pandas-read-csv | ||
| data = '''SEARCH_TERM,ACTUAL_URL | ||
| "bra tv bord","http://www.ikea.com/se/sv/catalog/categories/departments/living_room/10475/?se%7cps%7cnonbranded%7cvardagsrum%7cgoogle%7ctv_bord" | ||
| "bra tv board","http://www.ikea.com/se/sv/catalog/categories/departments/living_room/10475/?se%7cps%7cnonbranded%7cvardagsrum%7cgoogle%7ctv_bord" |
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.
ikea is swedish and "bra tv bord" translates as "good tv stand". :)
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.
Should I ignore this word then?
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.
naah, this is OK IMO, let's not ignore more than what's needed, I think this was written more in jest than anything 😄
|
Made all the necessary changes. |
|
I made the second PR with other files, as both of these PRs are merged I will make the final PR. |
|
Nice, thanks! There's some (unrelated to this PR) issue with CI though, will revisit once that's fixed |
|
Hi @01-vyom can you fetch and merge upstream/master please? |
…ndas-codespell-38802
|
Thanks @01-vyom ! |
Uh oh!
There was an error while loading. Please reload this page.