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.

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

Replaced !r with repr() #30020

Merged
merged 2 commits into from
Dec 11, 2019
Merged

Conversation

kvn219
Copy link
Contributor

@kvn219 kvn219 commented Dec 3, 2019

Issue: #29886

@WillAyd

@kvn219 kvn219 changed the title Replaced 'rm -rf pandas-kvn219' with repr() Replaced !r with repr() Dec 3, 2019
@alimcmaster1 alimcmaster1 added Code Style Code style, linting, code_checks Clean labels Dec 4, 2019
@WillAyd
Copy link
Member

WillAyd commented Dec 4, 2019

Thanks for the PR. Looks good and I think CI failures are unrelated / have been fixed on master since this was created.

To be sure, can you locally on your fstring-sorting branch run:

git fetch upstream
git merge upstream/master
git push origin fstring-sorting

I think should get CI green for this. Flag me down at PyData if you have any questions

Copy link
Member

@simonjayhawkins simonjayhawkins left a comment

Choose a reason for hiding this comment

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

@kvn219 lgtm pending green ci

@simonjayhawkins simonjayhawkins added this to the 1.0 milestone Dec 4, 2019
@@ -264,7 +264,7 @@ def nargsort(items, kind="quicksort", ascending: bool = True, na_position="last"
elif na_position == "first":
indexer = np.concatenate([nan_idx, indexer])
else:
raise ValueError("invalid na_position: {!r}".format(na_position))
raise ValueError(f"invalid na_position: {repr(na_position)}")
Copy link
Contributor

Choose a reason for hiding this comment

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

you can remove the repr on both of these as its unecessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback @jreback, I just pushed with repr() removed.

@WillAyd
Copy link
Member

WillAyd commented Dec 9, 2019

Looks like a merge conflict @kvn219 can you fix that up and re-push?

@kvn219
Copy link
Contributor Author

kvn219 commented Dec 11, 2019

Looks like a merge conflict @kvn219 can you fix that up and re-push?

@kvn219 kvn219 closed this Dec 11, 2019
@kvn219
Copy link
Contributor Author

kvn219 commented Dec 11, 2019

Hey @WillAyd, I'm not sure what to do. I'll go to HackLA next week and ask for help.

@kvn219 kvn219 reopened this Dec 11, 2019
@simonjayhawkins simonjayhawkins merged commit db60c79 into pandas-dev:master Dec 11, 2019
@simonjayhawkins
Copy link
Member

Thanks @kvn219

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Code Style Code style, linting, code_checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants