Skip to content

Commit

Permalink
[Fail on pytest warnings 1/n] Marking strings with invalid escape seq…
Browse files Browse the repository at this point in the history
…uences as raw strings (#31523)

Background
Python deprecated invalid unicode escape sequences a while back (Python3.6) and they will start breaking in newer Pythons:

Changed in version 3.6: Unrecognized escape sequences produce a DeprecationWarning. In a future Python version they will be a SyntaxWarning and eventually a SyntaxError.

Docstrings and other strings that need \  et al. should be raw strings.

Pytest?
For reasons unclear to me, pytest encounters the SyntaxError when it instruments tests with improved assertions when its warnings are interpreted as failures. It's unclear how to ignore them (because they're marked as SyntaxErrors, which aren't warnings and can't be filtered).

So, in this PR we fix the occurrences where we have invalid escape sequences. We have to do it in a separate PR from #31219 because our CI is using a prebuilt wheel instead of a per-PR wheel.

#31479 tracks the work to fail on pytest warnings.

Example offenders
For example, see lines 406, 407, 412, and 413 here:

ray/python/ray/data/grouped_dataset.py

Lines 406 to 413 in 0c8b59d

             ...     for i in range(100)]) \ # doctest: +SKIP 
             ...     .groupby(lambda x: x[0] % 3) \ # doctest: +SKIP 
             ...     .sum(lambda x: x[2]) # doctest: +SKIP 
             >>> ray.data.range_table(100).groupby("value").sum() # doctest: +SKIP 
             >>> ray.data.from_items([ # doctest: +SKIP 
             ...     {"A": i % 3, "B": i, "C": i**2} # doctest: +SKIP 
             ...     for i in range(100)]) \ # doctest: +SKIP 
             ...     .groupby("A") \ # doctest: +SKIP 

Signed-off-by: Cade Daniel <cade@anyscale.com>
  • Loading branch information
cadedaniel authored Jan 11, 2023
1 parent 6bbd5d8 commit e54ff46
Show file tree
Hide file tree
Showing 13 changed files with 31 additions and 11 deletions.
10 changes: 10 additions & 0 deletions pytest.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[pytest]

filterwarnings =
# This means: treat every warning as error, except for warnings that match .* (aka all warnings), ignore those.
# This triggers the SyntaxError described in https://github.com/ray-project/ray/pull/31523 but keeps the status quo
# warning behavior until https://github.com/ray-project/ray/pull/31219 .
error

# The format is `action:message_regex:category:module:line`.
ignore:.*:
2 changes: 1 addition & 1 deletion python/ray/_private/runtime_env/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def exec_worker(self, passthrough_args: List[str], language: Language):
else:
executable = "exec "

passthrough_args = [s.replace(" ", "\ ") for s in passthrough_args]
passthrough_args = [s.replace(" ", r"\ ") for s in passthrough_args]
exec_command = " ".join([f"{executable}"] + passthrough_args)
command_str = " ".join(self.command_prefix + [exec_command])
# TODO(SongGuyang): We add this env to command for macOS because it doesn't
Expand Down
2 changes: 1 addition & 1 deletion python/ray/air/util/tensor_extensions/pandas.py
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ def name(self) -> str:

@classmethod
def construct_from_string(cls, string: str):
"""
r"""
Construct this type from a string.
This is useful mainly for data types that accept parameters.
Expand Down
2 changes: 1 addition & 1 deletion python/ray/data/grouped_dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ def count(self) -> Dataset[U]:
def sum(
self, on: Union[KeyFn, List[KeyFn]] = None, ignore_nulls: bool = True
) -> Dataset[U]:
"""Compute grouped sum aggregation.
r"""Compute grouped sum aggregation.
This is a blocking operation.
Expand Down
3 changes: 3 additions & 0 deletions python/ray/tests/test_client_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,9 @@ def has_client_deprecation_warn(warning: Warning, expected_replacement: str) ->
@pytest.mark.skipif(
sys.platform == "win32", reason="pip not supported in Windows runtime envs."
)
@pytest.mark.filterwarnings(
"default:Starting a connection through `ray.client` will be deprecated"
)
def test_client_deprecation_warn():
"""
Tests that calling ray.client directly raises a deprecation warning with
Expand Down
7 changes: 7 additions & 0 deletions python/ray/tests/test_placement_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,7 @@ def test_placement_group_empty_bundle_error(ray_start_regular, connect_to_client
ray.util.placement_group([])


@pytest.mark.filterwarnings("default:placement_group parameter is deprecated")
def test_placement_group_scheduling_warning(ray_start_regular_shared):
@ray.remote
class Foo:
Expand Down Expand Up @@ -557,6 +558,12 @@ def foo():
assert not w


@pytest.mark.filterwarnings(
"default:Setting 'object_store_memory' for actors is deprecated"
)
@pytest.mark.filterwarnings(
"default:Setting 'object_store_memory' for bundles is deprecated"
)
def test_object_store_memory_deprecation_warning(ray_start_regular_shared):
with warnings.catch_warnings(record=True) as w:

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""
r"""
Test that focuses on wide fanout of deployment graph
-> Node_1
/ \
Expand Down Expand Up @@ -56,7 +56,7 @@ def combine(value_refs):
def test_wide_fanout_deployment_graph(
fanout_degree, init_delay_secs=0, compute_delay_secs=0
):
"""
r"""
Test that focuses on wide fanout of deployment graph
-> Node_1
/ \
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""
r"""
This test is parity of
release/serve_tests/workloads/deployment_graph_wide_ensemble.py
Instead of using graph api, the test is using pure handle to
Expand Down
2 changes: 1 addition & 1 deletion rllib/algorithms/algorithm_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2301,7 +2301,7 @@ def get_multi_agent_setup(
spaces: Optional[Dict[PolicyID, Tuple[Space, Space]]] = None,
default_policy_class: Optional[Type[Policy]] = None,
) -> Tuple[MultiAgentPolicyConfigDict, Callable[[PolicyID, SampleBatchType], bool]]:
"""Compiles complete multi-agent config (dict) from the information in `self`.
r"""Compiles complete multi-agent config (dict) from the information in `self`.
Infers the observation- and action spaces, the policy classes, and the policy's
configs. The returned `MultiAgentPolicyConfigDict` is fully unified and strictly
Expand Down
2 changes: 1 addition & 1 deletion rllib/algorithms/dqn/dqn.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@


class DQNConfig(SimpleQConfig):
"""Defines a configuration class from which a DQN Algorithm can be built.
r"""Defines a configuration class from which a DQN Algorithm can be built.
Example:
>>> from ray.rllib.algorithms.dqn.dqn import DQNConfig
Expand Down
2 changes: 1 addition & 1 deletion rllib/algorithms/marwil/marwil.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ def build(
) -> "Algorithm":
if not self._set_off_policy_estimation_methods:
deprecation_warning(
old="MARWIL used to have off_policy_estimation_methods "
old=r"MARWIL used to have off_policy_estimation_methods "
"is and wis by default. This has"
"changed to off_policy_estimation_methods: \{\}."
"If you want to use an off-policy estimator, specify it in"
Expand Down
2 changes: 1 addition & 1 deletion rllib/algorithms/mbmpo/mbmpo.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@


class MBMPOConfig(AlgorithmConfig):
"""Defines a configuration class from which an MBMPO Algorithm can be built.
r"""Defines a configuration class from which an MBMPO Algorithm can be built.
Example:
>>> from ray.rllib.algorithms.mbmpo import MBMPOConfig
Expand Down
2 changes: 1 addition & 1 deletion rllib/algorithms/r2d2/r2d2.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@


class R2D2Config(DQNConfig):
"""Defines a configuration class from which a R2D2 Algorithm can be built.
r"""Defines a configuration class from which a R2D2 Algorithm can be built.
Example:
>>> from ray.rllib.algorithms.r2d2.r2d2 import R2D2Config
Expand Down

0 comments on commit e54ff46

Please sign in to comment.