Skip to content

Conversation

@lexy-lixinyu
Copy link
Contributor

@alimcmaster1 alimcmaster1 added Clean Code Style Code style, linting, code_checks labels Dec 4, 2019
Copy link
Member

@WillAyd WillAyd 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 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 shiny-new-feature branch run:

git fetch upstream
git merge upstream/master
git push origin shiny-new-feature

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

pandas/io/sql.py Outdated
"Length of 'index_label' should match number of "
"levels, which is {0}".format(nlevels)
f"Length of 'index_label' should match number of "
"levels, which is {nlevels}"
Copy link
Member

Choose a reason for hiding this comment

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

this line needs an f

pandas/io/sql.py Outdated
"The type of {column} is not a "
"SQLAlchemy type ".format(column=col)
)
raise ValueError(f"The type of {col} is not a " "SQLAlchemy type ")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
raise ValueError(f"The type of {col} is not a " "SQLAlchemy type ")
raise ValueError(f"The type of {col} is not a SQLAlchemy type ")

Copy link
Member

Choose a reason for hiding this comment

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

remove trailing space?

pandas/io/sql.py Outdated
ex = DatabaseError(
"Execution failed on sql: {sql}\n{exc}\nunable "
"to rollback".format(sql=args[0], exc=exc)
f"Execution failed on sql: {args[0]}\n{exc}\nunable " "to rollback"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
f"Execution failed on sql: {args[0]}\n{exc}\nunable " "to rollback"
f"Execution failed on sql: {args[0]}\n{exc}\nunable to rollback"

pandas/io/sql.py Outdated
column=col, type=my_type
)
)
raise ValueError(f"{col} ({my_type!s}) not a string")
Copy link
Member

Choose a reason for hiding this comment

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

Is the !s required here?

@jreback jreback added this to the 1.0 milestone Dec 4, 2019
@WillAyd
Copy link
Member

WillAyd commented Dec 14, 2019

@lexy-lixinyu can you commit changes as suggested by @simonjayhawkins ?

@jbrockmendel
Copy link
Member

just pushed edits removing unintentional " " in two places.

@datapythonista
Copy link
Member

Thanks for the work on this @lexy-lixinyu, can you address the two last comments, and update your branch from master please? This is ready to be merged after it.

If you need help let us know.

@jbrockmendel
Copy link
Member

just pushed edits to address outstanding comments

Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

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

lgtm

@simonjayhawkins simonjayhawkins merged commit cfffad9 into pandas-dev:master Dec 31, 2019
@simonjayhawkins
Copy link
Member

Thanks @lexy-lixinyu and @jbrockmendel

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.

7 participants