Skip to content

Commit

Permalink
Remove compatibility shims for Focal (RobotLocomotion#20965)
Browse files Browse the repository at this point in the history
This reverts commit 6b32a81.
  • Loading branch information
jwnimmer-tri authored Feb 22, 2024
1 parent 6eb2d5b commit efc8831
Show file tree
Hide file tree
Showing 28 changed files with 55 additions and 183 deletions.
24 changes: 2 additions & 22 deletions bindings/pydrake/common/cpp_param.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,32 +142,12 @@ def __repr__(self):
return f"<Generic {self._name}>"


def _has_pep585():
return sys.version_info[:2] >= (3, 9)


def _dict_instantiator(params):
# Backport PEP-585 support into Python 3.8 and earlier.
if _has_pep585():
result = dict[params]
else:
result = typing.Dict[params]
# We need the result to be a callable type to match PEP-585 (i.e.,
# calling it must return a fresh `dict` object.)
result._inst = True
return result
return dict[params]


def _list_instantiator(params):
# Backport PEP-585 support into Python 3.8 and earlier.
if _has_pep585():
result = list[params]
else:
result = typing.List[params]
# We need the result to be a callable type to match PEP-585 (i.e.,
# calling it must return a fresh `list` object.)
result._inst = True
return result
return list[params]


def _optional_instantiator(params):
Expand Down
6 changes: 1 addition & 5 deletions bindings/pydrake/common/test/cpp_template_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,7 @@ def instantiation_func(param):

with self.assertRaises(TypeError) as cm:
assert_pickle(self, template)
if sys.version_info[:2] >= (3, 8):
pickle_error = "cannot pickle 'module' object"
else:
pickle_error = "can't pickle module objects"
self.assertIn(pickle_error, str(cm.exception))
self.assertIn("cannot pickle 'module' object", str(cm.exception))

def test_base_negative(self):

Expand Down
13 changes: 2 additions & 11 deletions bindings/pydrake/common/test/serialize_pybind_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,6 @@ def test_attributes_using_serialize_types(self):
self.assertEqual(dut.some_map, {'new_key': 70})
self.assertEqual(dut.some_variant, 80.0)

try:
# Use the PEP-585 types if supported (Python >= 3.9).
expected_vector_type = list[float]
expected_dict_type = dict[str, float]
except TypeError:
# Otherwise, use the Python 3.8 types.
expected_vector_type = typing.List[float]
expected_dict_type = typing.Dict[str, float]

# Check all field types.
fields = getattr(MyData2, "__fields__")
self.assertSequenceEqual([(x.name, x.type) for x in fields], (
Expand All @@ -119,8 +110,8 @@ def test_attributes_using_serialize_types(self):
("some_string", str),
("some_eigen", np.ndarray),
("some_optional", typing.Optional[float]),
("some_vector", expected_vector_type),
("some_map", expected_dict_type),
("some_vector", list[float]),
("some_map", dict[str, float]),
("some_variant", typing.Union[float, MyData1])))

def test_repr_using_serialize_no_docs(self):
Expand Down
5 changes: 2 additions & 3 deletions bindings/pydrake/common/test/yaml_typed_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,8 @@ class OptionalStructNoDefault:

@dc.dataclass
class NumpyStruct:
# TODO(jwnimmer-tri) Once we drop support for Ubuntu 20.04 "Focal", then we
# can upgrade to numpy >= 1.21 as our minimum at which point we can use the
# numpy.typing module here to constrain the shape and/or dtype.
# TODO(jwnimmer-tri) We should use the numpy.typing module here to
# constrain the shape and/or dtype.
value: np.ndarray = dc.field(
default_factory=lambda: np.array([nan]))

Expand Down
8 changes: 3 additions & 5 deletions bindings/pydrake/common/yaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -637,11 +637,9 @@ def _yaml_dump_typed_item(*, obj, schema):

# Handle NumPy types.
if schema == np.ndarray:
# TODO(jwnimmer-tri) Once we drop support for Ubuntu 20.04 "Focal",
# then we can upgrade to numpy >= 1.21 as our minimum at which point
# we can use the numpy.typing module here to statically specify a
# shape and/or dtype in the schema. Until then, we only support floats
# with no restrictions on the shape.
# TODO(jwnimmer-tri) We should use the numpy.typing module here to
# statically specify a shape and/or dtype in the schema. For now,
# we only support floats with no restrictions on the shape.
assert obj.dtype == np.dtype(np.float64)
list_value = obj.tolist()
list_schema = float
Expand Down
4 changes: 0 additions & 4 deletions bindings/pydrake/gym/_drake_gym_env.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import sys
from typing import Callable, Optional, Union
import warnings

Expand All @@ -18,9 +17,6 @@
from pydrake.systems.sensors import ImageRgba8U


assert sys.version_info >= (3, 10), "pydrake.gym requires Python 3.10 or newer"


class DrakeGymEnv(gym.Env):
"""
DrakeGymEnv provides a gym.Env interface for a Drake System (often a
Expand Down
8 changes: 1 addition & 7 deletions bindings/pydrake/systems/test/custom_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,7 @@ def DoGetGraphvizFragment(self, params):
# N.B. We cannot use `header_lines.append(...)` here; the property
# getter returns a _copy_ of the lines, not a _reference_.
params.header_lines += ["hello=world"]
if sys.version_info[:2] >= (3, 9):
params.options |= {"split": "I/O"}
else:
# TODO(jwnimmer-tri) Kill this spelling when we drop Ubuntu 20.04.
options = params.options
options.update({"split": "I/O"})
params.options = options
params.options |= {"split": "I/O"}
return super().DoGetGraphvizFragment(params)


Expand Down
2 changes: 1 addition & 1 deletion bindings/pydrake/visualization/test/meldis_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
#
# TODO(mwoehlke-kitware): Remove this when Jammy's python3-u-msgpack has been
# updated to 2.5.2 or later.
if sys.version_info[:2] >= (3, 10) and not hasattr(umsgpack, 'Hashable'):
if not hasattr(umsgpack, 'Hashable'):
import collections
setattr(umsgpack.collections, 'Hashable', collections.abc.Hashable)

Expand Down
53 changes: 6 additions & 47 deletions common/overloaded.h
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
#pragma once

#include <utility>
#include <variant>

/** @file
The "overloaded" variant-visit pattern.
Expand All @@ -11,8 +8,8 @@ doesn't support it natively. There is a commonly used two-line boilerplate
that bridges this gap; see
https://en.cppreference.com/w/cpp/utility/variant/visit
This file should be included by classes that wish to use the variant visit
pattern, i.e.:
This file should be included by classes that wish to
use the variant visit pattern, i.e.
@code {.cpp}
using MyVariant = std::variant<int, std::string>;
Expand All @@ -24,23 +21,6 @@ pattern, i.e.:
EXPECT_EQ(result, "found an int");
@endcode
However, note that the prior example DOES NOT WORK yet in Drake. In order to
support C++17 we must also (for now) use a polyfill for `std::visit<T>` which
we've named `visit_overloaded<T>`, so within Drake we must spell it like this:
@code {.cpp}
using MyVariant = std::variant<int, std::string>;
MyVariant v = 5;
std::string result = visit_overloaded<const char*>(overloaded{
[](const int arg) { return "found an int"; },
[](const std::string& arg) { return "found a string"; }
}, v);
EXPECT_EQ(result, "found an int");
@endcode
When we drop support for C++17, we'll be able to return back to the normal
pattern.
@warning This file must never be included by a header, only by a cc file.
This is enforced by a lint rule in `tools/lint/drakelint.py`.
*/
Expand All @@ -55,32 +35,11 @@ template <class... Ts>
struct overloaded : Ts... {
using Ts::operator()...;
};

// Even though Drake requires C++20, Clang versions prior to 17 do not implement
// P1816 yet, so we need to write out a deduction guide. We can remove this once
// Drake requires Clang 17 or newer (as well as the matching Apple Clang).
template <class... Ts>
overloaded(Ts...) -> overloaded<Ts...>;

// NOTE: The second line above can be removed when we are compiling with
// >= C++20 on all platforms.

// This is a polyfill for C++20's std::visit<Return>(visitor, variant) that we
// need while we still support C++17. Once we drop C++17 (i.e., once we drop
// Ubuntu 20.04), we should switch back to the conventional spelling and remove
// this entire block of code.
#if __cplusplus > 201703L
// On reasonable platforms, we can just call std::visit.
template <typename Return, typename Visitor, typename Variant>
auto visit_overloaded(Visitor&& visitor, Variant&& variant) -> decltype(auto) {
return std::visit<Return>(std::forward<Visitor>(visitor),
std::forward<Variant>(variant));
}
#else
// On Focal, we need to do a polyfill.
template <typename Return, typename Visitor, typename Variant>
auto visit_overloaded(Visitor&& visitor, Variant&& variant) -> Return {
auto visitor_coerced = [&visitor]<typename Value>(Value&& value) -> Return {
return visitor(std::forward<Value>(value));
};
return std::visit(visitor_coerced, std::forward<Variant>(variant));
}
#endif

} // namespace
4 changes: 2 additions & 2 deletions common/schema/rotation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Rotation::Rotation(const math::RollPitchYaw<double>& arg) {
}

bool Rotation::IsDeterministic() const {
return visit_overloaded<bool>(overloaded{
return std::visit<bool>(overloaded{
[](const Identity&) {
return true;
},
Expand Down Expand Up @@ -65,7 +65,7 @@ Vector<Expression, Size> deg2rad(

math::RotationMatrix<Expression> Rotation::ToSymbolic() const {
using Result = math::RotationMatrix<Expression>;
return visit_overloaded<Result>(overloaded{
return std::visit<Result>(overloaded{
[](const Identity&) {
return Result{};
},
Expand Down
6 changes: 3 additions & 3 deletions common/schema/stochastic.cc
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ drake::VectorX<Expression> ToSymbolic(
}

bool IsDeterministic(const DistributionVariant& var) {
return visit_overloaded<bool>(overloaded{
return std::visit<bool>(overloaded{
[](const double&) {
return true;
},
Expand Down Expand Up @@ -350,7 +350,7 @@ drake::VectorX<Expression> UniformVector<Size>::ToSymbolic() const {
template <int Size>
unique_ptr<DistributionVector> ToDistributionVector(
const DistributionVectorVariant<Size>& vec) {
return visit_overloaded<unique_ptr<DistributionVector>>(overloaded{
return std::visit<unique_ptr<DistributionVector>>(overloaded{
// NOLINTNEXTLINE(whitespace/line_length)
[](const drake::Vector<double, Size>& arg) {
return std::make_unique<DeterministicVector<Size>>(arg);
Expand Down Expand Up @@ -387,7 +387,7 @@ unique_ptr<DistributionVector> ToDistributionVector(

template <int Size>
bool IsDeterministic(const DistributionVectorVariant<Size>& vec) {
return visit_overloaded<bool>(overloaded{
return std::visit<bool>(overloaded{
[](const drake::Vector<double, Size>&) {
return true;
},
Expand Down
6 changes: 3 additions & 3 deletions common/test/overloaded_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ GTEST_TEST(OverloadedTest, CommentExampleTest) {
// Test the exact text of the example in the file comment.
using MyVariant = std::variant<int, std::string>;
MyVariant v = 5;
std::string result = visit_overloaded<const char*>(overloaded{
std::string result = std::visit<const char*>(overloaded{
[](const int arg) { return "found an int"; },
[](const std::string& arg) { return "found a string"; }
}, v);
Expand All @@ -24,7 +24,7 @@ GTEST_TEST(OverloadedTest, AutoTest) {

// An 'auto' arm doesn't match if there's any explicit match,
// no matter if it's earlier or later in the list.
std::string result = visit_overloaded<const char*>(overloaded{
std::string result = std::visit<const char*>(overloaded{
[](const auto arg) { return "found an auto"; },
[](const int arg) { return "found an int"; },
[](const std::string& arg) { return "found a string"; },
Expand All @@ -33,7 +33,7 @@ GTEST_TEST(OverloadedTest, AutoTest) {
EXPECT_EQ(result, "found an int");

// An 'auto' arm matches if there's no explicit match.
result = visit_overloaded<const char*>(overloaded{
result = std::visit<const char*>(overloaded{
[](const auto arg) { return "found an auto"; },
[](const std::string& arg) { return "found a string"; },
}, v);
Expand Down
6 changes: 1 addition & 5 deletions common/value.h
Original file line number Diff line number Diff line change
Expand Up @@ -401,18 +401,14 @@ constexpr bool hash_template_argument_from_pretty_func(
return true;
}

// Akin to C++17 std::void_t<>.
template <typename...>
using typehasher_void_t = void;

// Traits type to ask whether T::NonTypeTemplateParameter exists.
template <typename T, typename U = void>
struct TypeHasherHasNonTypeTemplateParameter {
static constexpr bool value = false;
};
template <typename T>
struct TypeHasherHasNonTypeTemplateParameter<
T, typehasher_void_t<typename T::NonTypeTemplateParameter>> {
T, std::void_t<typename T::NonTypeTemplateParameter>> {
static constexpr bool value = true;
};

Expand Down
Loading

0 comments on commit efc8831

Please sign in to comment.