Skip to content

Commit

Permalink
respond to review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
cosmicexplorer committed Jan 21, 2019
1 parent 427fb2f commit da9bfcb
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 9 deletions.
21 changes: 12 additions & 9 deletions src/python/pants/util/dirutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ def safe_mkdir_for_all(paths):
created_dirs.add(dir_to_make)


@deprecated('1.16.0.dev2', hint_message='Use safe_file_write() instead!')
@deprecated(
'1.16.0.dev2',
hint_message='Use safe_file_write() instead. It removes the deprecated `binary_mode` argument to instead only allow `mode`. It also defaults to writing unicode, rather than defaulting to bytes.')
def safe_file_dump(filename, payload, binary_mode=None, mode=None):
"""Write a string to a file.
Expand Down Expand Up @@ -130,7 +132,10 @@ def safe_file_dump(filename, payload, binary_mode=None, mode=None):
safe_file_write(filename, payload=payload, mode=mode)


def safe_file_write(filename, payload=None, mode=None):
# TODO(#6742): payload should be Union[str, bytes] in type hint syntax, but from
# https://pythonhosted.org/an_example_pypi_project/sphinx.html#full-code-example it doesn't appear
# that is possible to represent in docstring type syntax.
def safe_file_write(filename, payload='', mode='w'):
"""Write a string to a file.
This method is "safe" to the extent that `safe_open` is "safe". See the explanation on the method
Expand All @@ -140,12 +145,10 @@ def safe_file_write(filename, payload=None, mode=None):
create an empty file along with its containing directory (or truncate it if it already exists).
:param string filename: The filename of the file to write to.
:param string payload: The string to write to the file. Defaults to the empty string.
:param string mode: A mode argument for the python `open` builtin. Defaults to 'w' (text).
:param string payload: The string to write to the file.
:param string mode: A mode argument for the python `open` builtin.
"""
if payload is None:
payload = ''
with safe_open(filename, mode=mode or 'w') as f:
with safe_open(filename, mode=mode) as f:
f.write(payload)


Expand All @@ -162,7 +165,7 @@ def maybe_read_file(filename, binary_mode=None):
lambda: binary_mode is None,
removal_version='1.16.0.dev2',
entity_description='Not specifying binary_mode explicitly in maybe_read_file()',
hint_message='This will default to unicode when pants migrates to python 3!')
hint_message='Function will default to unicode when pants migrates to python 3!')
if binary_mode is None:
binary_mode = True

Expand All @@ -185,7 +188,7 @@ def read_file(filename, binary_mode=None):
lambda: binary_mode is None,
removal_version='1.16.0.dev2',
entity_description='Not specifying binary_mode explicitly in read_file()',
hint_message='This will default to unicode when pants migrates to python 3!')
hint_message='Function will default to unicode when pants migrates to python 3!')
if binary_mode is None:
binary_mode = True

Expand Down
2 changes: 2 additions & 0 deletions tests/python/pants_test/util/test_dirutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,8 @@ def test_rm_rf_no_such_file_not_an_error(self, file_name='./vanishing_file'):
def assert_write_and_read(self, test_content, write_kwargs, read_kwargs):
with temporary_dir() as td:
test_filename = os.path.join(td, 'test.out')
# TODO: remove all tests of safe_file_dump() and convert the relevant ones to
# safe_file_write() after the deprecation period is over!
safe_file_dump(test_filename, test_content, **write_kwargs)
self.assertEqual(read_file(test_filename, **read_kwargs), test_content)

Expand Down

0 comments on commit da9bfcb

Please sign in to comment.