-
Notifications
You must be signed in to change notification settings - Fork 49
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
avoid tomli Text file object support is deprecated and enable pytest strict mode #122
Conversation
pyproject.toml
Outdated
junit_family = "xunit2" | ||
filterwarnings = [ | ||
"error", | ||
'''ignore:Text file object support is deprecated in favor of binary file objects\. Use `open\("foo.toml", "rb"\)` to open the file in binary mode\.:DeprecationWarning''', |
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.
this deprecation warning is causing downstream failures:
https://github.com/pypa/setuptools/pull/2746/checks?check_run_id=3200086380#step:5:339
pep517/compat.py
Outdated
from toml import load as _toml_load # noqa: F401 | ||
|
||
def toml_load(f): | ||
w = io.TextIOWrapper(f, encoding="utf8") |
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.
@hukkin do I need this?
w = io.TextIOWrapper(f, encoding="utf8") | |
w = io.TextIOWrapper(f, encoding="utf8", newline="") |
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.
Yes, encoding="utf8", newline=""
are the correct parameters for opening a TOML file in text mode.
The load
function from uiri/toml seems like it should support binary file objects though, so I'm not sure that any of this def toml_load
is needed?
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.
================================================================================================ FAILURES =================================================================================================
__________________________________________________________________________________ test_missing_backend_gives_exception ___________________________________________________________________________________
def test_missing_backend_gives_exception():
> hooks = get_hooks('pkg1')
tests/test_call_hooks.py:40:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/test_call_hooks.py:33: in get_hooks
data = toml_load(f)
.tox/py35/lib/python3.5/site-packages/toml/decoder.py:156: in load
return loads(f.read(), _dict, decoder)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
s = b'[build-system]\nrequires = ["eg_buildsys"]\nbuild-backend = "buildsys"\n\n[project]\ndescription = "Factory \xe2\xb8\xbb A code generator \xf0\x9f\x8f\xad"\nmaintainers = [{name = "\xc5\x81ukasz Langa"}]\n'
_dict = <class 'dict'>, decoder = <toml.decoder.TomlDecoder object at 0x7f9c930ef278>
def loads(s, _dict=dict, decoder=None):
"""Parses string as toml
Args:
s: String to be parsed
_dict: (optional) Specifies the class of the returned toml dictionary
Returns:
Parsed toml file represented as a dictionary
Raises:
TypeError: When a non-string is passed
TomlDecodeError: Error while decoding toml
"""
implicitgroups = []
if decoder is None:
decoder = TomlDecoder(_dict)
retval = decoder.get_empty_table()
currentlevel = retval
if not isinstance(s, basestring):
> raise TypeError("Expecting something like a string")
E TypeError: Expecting something like a string
.tox/py35/lib/python3.5/site-packages/toml/decoder.py:186: TypeError
======================================================================
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.
basestring is a str
not a (bytes, str)
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.
Ah I see, seems like a Python 3 specific bug.
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.
Given that we always use it with a real file anyway, we could make a compat function that takes a path to a file instead of a file like object. I'm fine with this way if you prefer, though.
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.
Hey there, I currently can't run CI builds on this project so would prefer to do changes in a followup PR, if you're happy either way
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.
Sure, you don't have to do it at all - it's just an idea. I'm not sure what you mean about CI, though - it seems to have run fine.
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've been scrounging build permissions from pypa discord
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.
Aha, gotcha.
This won't work with older versions of tomli, will it? Binary file support was added in 1.1.0. |
Bumped |
Sorry to see that toml is going back to Python 1 days. This patch seems good to me and I'd like to see it fixed as it's affecting other projects too. I'm less sure about enabling strict mode. |
Co-authored-by: Jason R. Coombs <jaraco@jaraco.com>
No description provided.