Skip to content

bpo-24024: update str.__doc__ to show default encoding explicitly #257

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

Closed
wants to merge 4 commits into from

Conversation

plusminushalf
Copy link
Contributor

@plusminushalf plusminushalf commented Feb 23, 2017

sys.getdefaultencoding() always returns 'utf-8'
Reference: Objects/unicodeobject.c line number -> 4214

Also str() accepts bytes, bytearray or buffer-like objects as input
hence changing it to object.
As str returns .__str__() method defined inside object or repr(obj)
otherwise.

sys.getdefaultencoding() always returns 'utf-8'
Reference: Objects/unicodeobject.c line number -> 4214

Also str() accepts bytes, bytearray or buffer-like objects as input
hence changing it to object.
As str returns .__str__() method defined inside object or repr(obj)
otherwise.
@plusminushalf
Copy link
Contributor Author

>>> str.__doc__
'str(object=\'\') -> str\nstr(object, encoding="utf-8", errors="strict") -> str\n\nCreate a new string object from the given object. If encoding or\nerrors is specified, then the object must expose a data buffer\nthat will be decoded using the given encoding and error handler.\nOtherwise, returns the result of object.__str__() (if defined)\nor repr(object).\nencoding defaults to sys.getdefaultencoding().\nerrors defaults to \'strict\'.'

@@ -15120,7 +15120,7 @@ unicode_subtype_new(PyTypeObject *type, PyObject *args, PyObject *kwds)

PyDoc_STRVAR(unicode_doc,
"str(object='') -> str\n\
str(bytes_or_buffer[, encoding[, errors]]) -> str\n\
str(object, encoding=\"utf-8\", errors=\"strict\") -> str\n\
Copy link
Member

Choose a reason for hiding this comment

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

Best to use single quotes to be consistent with the line above.

Since you add the defaults to the signature, I think you can remove them from the end of the text below.

Copy link
Member

@vadmium vadmium left a comment

Choose a reason for hiding this comment

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

In general this looks worthwhile to me, just some minor suggestions

1. Changing double quotes to single
2. Removing defaults information from the end since it is already
defined in function signature
@@ -15120,15 +15120,13 @@ unicode_subtype_new(PyTypeObject *type, PyObject *args, PyObject *kwds)

PyDoc_STRVAR(unicode_doc,
"str(object='') -> str\n\
str(bytes_or_buffer[, encoding[, errors]]) -> str\n\
str(object, encoding='utf-8', errors='strict') -> str\n\
Copy link
Contributor

@DimitrisJim DimitrisJim Apr 4, 2017

Choose a reason for hiding this comment

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

I'm not seeing why this needs two signatures now when str(object='', encoding='utf-8', errors='strict') would suffice.

Copy link
Member

Choose a reason for hiding this comment

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

I don’t think that merging the two lines is correct.

@@ -15119,8 +15119,7 @@ unicode_subtype_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
}

PyDoc_STRVAR(unicode_doc,
"str(object='') -> str\n\
str(object, encoding='utf-8', errors='strict') -> str\n\
"str(object, encoding='utf-8', errors='strict') -> str\n\
Copy link
Member

Choose a reason for hiding this comment

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

Passing no arguments is also supported:

>>> str()
''

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, object needs a default value, i.e object=''.

@codecov
Copy link

codecov bot commented Apr 10, 2017

Codecov Report

Merging #257 into master will decrease coverage by 1%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #257      +/-   ##
==========================================
- Coverage   83.38%   82.37%   -1.01%     
==========================================
  Files        1367     1428      +61     
  Lines      344811   351129    +6318     
==========================================
+ Hits       287516   289255    +1739     
- Misses      57295    61874    +4579

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4c78c52...d860f4b. Read the comment docs.

@@ -15119,16 +15119,13 @@ unicode_subtype_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
}

PyDoc_STRVAR(unicode_doc,
"str(object='') -> str\n\
str(bytes_or_buffer[, encoding[, errors]]) -> str\n\
"str(object='', encoding='utf-8', errors='strict') -> str\n\
Copy link
Member

Choose a reason for hiding this comment

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

There is a problem with this. This signature means that str('', 'utf-8', 'strict') is equivalent to str(). But calling str with string argument and encoding is an error. The signature of str can't be expressed in such form, this is a cause why the str constructor still is not converted to Argument Clinic.

Copy link
Member

Choose a reason for hiding this comment

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

I don’t think that merging the two lines is correct.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

Sorry, it was approved by mistake. I don't approve this patch in the current form.

@brettcannon
Copy link
Member

To try and help move older pull requests forward, we are going through and backfilling 'awaiting' labels on pull requests that are lacking the label. Based on the current reviews, the best we can tell in an automated fashion is that a core developer requested changes to be made to this pull request.

If/when the requested changes have been made, please leave a comment that says, I have made the requested changes; please review again. That will trigger a bot to flag this pull request as ready for a follow-up review.

@serhiy-storchaka
Copy link
Member

Closed because it make the docstring incorrect.

akruis pushed a commit to akruis/cpython that referenced this pull request Jun 4, 2021
Adapt patchcheck.py to the new name of the development branch.
jaraco pushed a commit that referenced this pull request Dec 2, 2022
jaraco added a commit to jaraco/cpython that referenced this pull request Feb 17, 2023
… have features/fixes present in this library. Fixes python#257.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants