Skip to content

used f-strings in docs #32133

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
Merged
merged 1 commit into from
Feb 22, 2020
Merged

used f-strings in docs #32133

merged 1 commit into from
Feb 22, 2020

Conversation

PSY27
Copy link
Contributor

@PSY27 PSY27 commented Feb 20, 2020 •
edited by jreback
Loading

rohitsanj
rohitsanj suggested changes Feb 20, 2020
View reviewed changes
Copy link
Contributor

@rohitsanj rohitsanj left a comment

Choose a reason for hiding this comment

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

some comments for your consideration

doc/source/whatsnew/v0.10.0.rst Outdated
@@ -297,7 +297,7 @@ Updated PyTables support

.. code-block:: ipython

In [41]: store = pd.HDFStore('store.h5')
In [41]: store = pd.HDFStore('store.h5')
Copy link
Contributor

Choose a reason for hiding this comment

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

undo this change, remove the whitespace

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok
thanks for the input

@PSY27 PSY27 changed the title replaced string = "HolyMoly" instead of string = "HolyMoly'" as there is a typo in docs" replaced string = "HolyMoly" instead of string = "HolyMoly'" as there is a typo in docs" #32039 Feb 20, 2020
@PSY27 PSY27 changed the title replaced string = "HolyMoly" instead of string = "HolyMoly'" as there is a typo in docs" #32039 replaced string = "HolyMoly" instead of string = "HolyMoly'" as there is a typo in docs" Feb 20, 2020
@PSY27 PSY27 changed the title replaced string = "HolyMoly" instead of string = "HolyMoly'" as there is a typo in docs" replaced string = "HolyMoly" instead of string = "HolyMoly'" " Feb 20, 2020
@PSY27 PSY27 changed the title replaced string = "HolyMoly" instead of string = "HolyMoly'" " replaced string = "HolyMoly" instead of string = "HolyMoly'" Feb 20, 2020
@WillAyd
Copy link
Member

WillAyd commented Feb 20, 2020

I think this is correct as is - isn't this supposed to show how the single quote doesn't get escaped? See text immediately following

WillAyd
WillAyd requested changes Feb 20, 2020
View reviewed changes
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.

as mentioned let's revert the change to the string, but can use f-strings if you'd like to make a change

doc/source/user_guide/io.rst Outdated
.. code-block:: ipython

string = "HolyMoly'"
string = "HolyMoly"
store.select('df', 'index == %s' % string)
Copy link
Member

Choose a reason for hiding this comment

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

You can update this to use an f-string if you'd like to change something

@MarcoGorelli
Copy link
Member

MarcoGorelli commented Feb 21, 2020 •
edited
Loading

I think this is correct as is - isn't this supposed to show how the single quote doesn't get escaped? See text immediately following

Yup, my bad - sorry @PSY27 . Though as Will said, if you change line 3818 to use an f-string, that would be good. In that case, you could add

ref #29547

to the pull request description instead of

#32039

rohitsanj
rohitsanj suggested changes Feb 22, 2020
View reviewed changes
doc/source/user_guide/io.rst Outdated
.. code-block:: ipython

string = "HolyMoly'"
string = "HolyMoly"
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert this change as well

@PSY27
Copy link
Contributor Author

PSY27 commented Feb 22, 2020

@WillAyd @MarcoGorelli please review it now

@PSY27 PSY27 changed the title replaced string = "HolyMoly" instead of string = "HolyMoly'" used f-strings in docs Feb 22, 2020
MarcoGorelli
MarcoGorelli suggested changes Feb 22, 2020
View reviewed changes
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good, thanks @PSY27!

While we're here, there's some other .formats which could be changed within this file - for example, this one on line 3828:

store.select('df', f'index == {string!r}')

Else I think it's fine as is, I realise the requirement for this PR wasn't clear upfront :)

EDIT

Though if you do make this extra change (line 3828) please also change the line

If you *must* interpolate, use the ``'%r'`` format specifier

to

If you *must* interpolate, use the ``!r`` conversion flag

@jreback jreback added this to the 1.1 milestone Feb 22, 2020
@jreback jreback added the IO HDF5 read_hdf, HDFStore label Feb 22, 2020
@jreback jreback merged commit f9b49c8 into pandas-dev:master Feb 22, 2020
@jreback
Copy link
Contributor

jreback commented Feb 22, 2020

thanks @PSY27

roberthdevries pushed a commit to roberthdevries/pandas that referenced this pull request Mar 2, 2020
@PSY27 @roberthdevries
used f-string (pandas-dev#32133)
f6369e8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WillAyd WillAyd WillAyd requested changes

@MarcoGorelli MarcoGorelli MarcoGorelli requested changes

@rohitsanj rohitsanj rohitsanj requested changes

Assignees
No one assigned
Labels
Docs IO HDF5 read_hdf, HDFStore
Projects
None yet
Milestone
1.1
Development

Successfully merging this pull request may close these issues.
6 participants
@PSY27 @WillAyd @MarcoGorelli @jreback @rohitsanj @simonjayhawkins