Skip to content

Commit

Permalink
[parsing] Deprecate PopulateUpstreamToDrake
Browse files Browse the repository at this point in the history
  • Loading branch information
jwnimmer-tri committed Jan 25, 2022
1 parent 3ba8e89 commit 6aa2b9a
Show file tree
Hide file tree
Showing 12 changed files with 39 additions and 30 deletions.
5 changes: 4 additions & 1 deletion bindings/pydrake/multibody/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,10 @@ drake_pybind_library(

drake_pybind_library(
name = "parsing_py",
cc_deps = ["//bindings/pydrake:documentation_pybind"],
cc_deps = [
"//bindings/pydrake:documentation_pybind",
"//bindings/pydrake/common:deprecation_pybind",
],
cc_srcs = ["parsing_py.cc"],
package_info = PACKAGE_INFO,
py_deps = [
Expand Down
13 changes: 10 additions & 3 deletions bindings/pydrake/multibody/parsing_py.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "pybind11/pybind11.h"
#include "pybind11/stl.h"

#include "drake/bindings/pydrake/common/deprecation_pybind.h"
#include "drake/bindings/pydrake/documentation_pybind.h"
#include "drake/bindings/pydrake/pydrake_pybind.h"
#include "drake/multibody/parsing/package_map.h"
Expand All @@ -27,7 +28,8 @@ PYBIND11_MODULE(parsing, m) {
{
using Class = PackageMap;
constexpr auto& cls_doc = doc.PackageMap;
py::class_<Class>(m, "PackageMap", cls_doc.doc)
py::class_<Class> cls(m, "PackageMap", cls_doc.doc);
cls // BR
.def(py::init<>(), cls_doc.ctor.doc)
.def("Add", &Class::Add, py::arg("package_name"),
py::arg("package_path"), cls_doc.Add.doc)
Expand All @@ -48,9 +50,14 @@ PYBIND11_MODULE(parsing, m) {
.def("PopulateFromEnvironment", &Class::PopulateFromEnvironment,
py::arg("environment_variable"),
cls_doc.PopulateFromEnvironment.doc)
.def("PopulateUpstreamToDrake", &Class::PopulateUpstreamToDrake,
py::arg("model_file"), cls_doc.PopulateUpstreamToDrake.doc)
.def_static("MakeEmpty", &Class::MakeEmpty, cls_doc.MakeEmpty.doc);
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
cls.def("PopulateUpstreamToDrake",
WrapDeprecated(cls_doc.PopulateUpstreamToDrake.doc_deprecated,
&Class::PopulateUpstreamToDrake),
py::arg("model_file"), cls_doc.PopulateUpstreamToDrake.doc_deprecated);
#pragma GCC diagnostic pop
}

// Parser
Expand Down
4 changes: 3 additions & 1 deletion bindings/pydrake/multibody/test/parsing_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import unittest

from pydrake.common import FindResourceOrThrow
from pydrake.common.test_utilities.deprecation import catch_drake_warnings
from pydrake.multibody.tree import (
ModelInstanceIndex,
)
Expand Down Expand Up @@ -51,7 +52,8 @@ def test_package_map(self):
self.assertEqual(dut2.size(), 0)

# Simple coverage test for Drake paths.
dut.PopulateUpstreamToDrake(model_file=model)
with catch_drake_warnings(expected_count=1):
dut.PopulateUpstreamToDrake(model_file=model)
self.assertGreater(dut.size(), 1)

# Simple coverage test for folder and environment.
Expand Down
1 change: 0 additions & 1 deletion examples/pr2/test/load_pr2_simplified_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ GTEST_TEST(LoadPr2SimplifiedTest, TestIfPr2SimplifiedLoads) {
const std::string& pathname = FindResourceOrThrow(
"drake/examples/pr2/models/pr2_description/urdf/pr2_simplified.urdf");
multibody::Parser parser(&plant);
parser.package_map().PopulateUpstreamToDrake(pathname);
parser.AddModelFromFile(pathname);
plant.Finalize();

Expand Down
2 changes: 2 additions & 0 deletions multibody/parsing/package_map.cc
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,8 @@ void PackageMap::PopulateUpstreamToDrakeHelper(
GetParentDirectory(directory), stop_at_directory);
}

// N.B. When removing this deprecated function, also be sure to remove
// the PopulateUpstreamToDrakeHelper, immediately above.
void PackageMap::PopulateUpstreamToDrake(const string& model_file) {
DRAKE_DEMAND(!model_file.empty());
drake::log()->trace("PopulateUpstreamToDrake: {}", model_file);
Expand Down
3 changes: 3 additions & 0 deletions multibody/parsing/package_map.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <vector>

#include "drake/common/drake_copyable.h"
#include "drake/common/drake_deprecated.h"

namespace drake {
namespace multibody {
Expand Down Expand Up @@ -100,6 +101,8 @@ class PackageMap final {
///
/// @param[in] model_file The model file whose directory is the start of the
/// search for `package.xml` files. This file must be an SDF or URDF file.
DRAKE_DEPRECATED("2022-05-01",
"This function is a no-op; you may remove any calls to it.")
void PopulateUpstreamToDrake(const std::string& model_file);

friend std::ostream& operator<<(std::ostream& out,
Expand Down
6 changes: 6 additions & 0 deletions multibody/parsing/parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,12 @@ std::vector<ModelInstanceIndex> Parser::AddAllModelsFromFile(
const std::string& file_name) {
DataSource data_source;
data_source.file_name = &file_name;
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
// Always search for a package.xml file, starting the crawl upward from
// the file's path.
package_map_.PopulateUpstreamToDrake(file_name);
#pragma GCC diagnostic pop
const FileType type = DetermineFileType(file_name);
if (type == FileType::kSdf) {
return AddModelsFromSdf(
Expand All @@ -58,9 +61,12 @@ ModelInstanceIndex Parser::AddModelFromFile(
const std::string& model_name) {
DataSource data_source;
data_source.file_name = &file_name;
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
// Always search for a package.xml file, starting the crawl upward from
// the file's path.
package_map_.PopulateUpstreamToDrake(file_name);
#pragma GCC diagnostic pop
const FileType type = DetermineFileType(file_name);
if (type == FileType::kSdf) {
return AddModelFromSdf(
Expand Down
5 changes: 4 additions & 1 deletion multibody/parsing/test/detail_path_utils_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,10 @@ GTEST_TEST(ResolveUriTest, TestModel) {

// Create the package map.
PackageMap package_map;
package_map.PopulateUpstreamToDrake(sdf_file_name);
package_map.AddPackageXml(FindResourceOrThrow(
"drake/multibody/parsing/test/"
"package_map_test_packages/package_map_test_package_a/"
"package.xml"));

// Set the root directory - it will not end up being used in ResolveUri().
const std::string root_dir = "/no/such/root";
Expand Down
15 changes: 3 additions & 12 deletions multibody/parsing/test/detail_sdf_parser_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ GTEST_TEST(MultibodyPlantSdfParserTest, ModelInstanceTest) {
"drake/multibody/parsing/test/"
"links_with_visuals_and_collisions.sdf");
PackageMap package_map;
package_map.PopulateUpstreamToDrake(full_name);

ModelInstanceIndex instance1 =
AddModelFromSdfFile(full_name, "instance1", package_map, &plant);
Expand Down Expand Up @@ -652,7 +651,6 @@ GTEST_TEST(SdfParserThrowsWhen, JointDampingIsNegative) {
"drake/multibody/parsing/test/sdf_parser_test/"
"negative_damping_joint.sdf");
PackageMap package_map;
package_map.PopulateUpstreamToDrake(sdf_file_path);
MultibodyPlant<double> plant(0.0);
DRAKE_EXPECT_THROWS_MESSAGE(
AddModelFromSdfFile(sdf_file_path, "", package_map, &plant),
Expand Down Expand Up @@ -738,7 +736,6 @@ GTEST_TEST(SdfParser, TestOptionalSceneGraph) {
"drake/multibody/parsing/test/"
"links_with_visuals_and_collisions.sdf");
PackageMap package_map;
package_map.PopulateUpstreamToDrake(full_name);

int num_visuals_explicit{};
{
Expand Down Expand Up @@ -770,7 +767,6 @@ GTEST_TEST(MultibodyPlantSdfParserTest, JointParsingTest) {
"drake/multibody/parsing/test/sdf_parser_test/"
"joint_parsing_test.sdf");
PackageMap package_map;
package_map.PopulateUpstreamToDrake(full_name);

// Read in the SDF file.
const std::vector<ModelInstanceIndex> instances =
Expand Down Expand Up @@ -927,7 +923,6 @@ GTEST_TEST(MultibodyPlantSdfParserTest, JointActuatorParsingTest) {
"drake/multibody/parsing/test/sdf_parser_test/"
"joint_actuator_parsing_test.sdf");
PackageMap package_map;
package_map.PopulateUpstreamToDrake(full_name);

// Read in the SDF file.
AddModelFromSdfFile(full_name, "", package_map, &plant, nullptr);
Expand Down Expand Up @@ -962,7 +957,6 @@ GTEST_TEST(MultibodyPlantSdfParserTest, RevoluteSpringParsingTest) {
"drake/multibody/parsing/test/sdf_parser_test/"
"revolute_spring_parsing_test.sdf");
PackageMap package_map;
package_map.PopulateUpstreamToDrake(full_name);

// Reads in the SDF file.
AddModelFromSdfFile(full_name, "", package_map, &plant, nullptr);
Expand Down Expand Up @@ -1225,7 +1219,6 @@ ::testing::AssertionResult FrameHasShape(geometry::FrameId frame_id,
void TestForParsedGeometry(const char* sdf_name, geometry::Role role) {
const std::string full_name = FindResourceOrThrow(sdf_name);
PackageMap package_map;
package_map.PopulateUpstreamToDrake(full_name);
MultibodyPlant<double> plant(0.0);
SceneGraph<double> scene_graph;
plant.RegisterAsSourceForSceneGraph(&scene_graph);
Expand Down Expand Up @@ -1474,7 +1467,6 @@ GTEST_TEST(SdfParser, LoadDirectlyNestedModelsInWorld) {
ASSERT_EQ(plant.num_joints(), 0);

PackageMap package_map;
package_map.PopulateUpstreamToDrake(full_name);
AddModelsFromSdfFile(full_name, package_map, &plant);
plant.Finalize();

Expand Down Expand Up @@ -1533,7 +1525,6 @@ GTEST_TEST(SdfParser, LoadDirectlyNestedModelsInModel) {
ASSERT_EQ(plant.num_joints(), 0);

PackageMap package_map;
package_map.PopulateUpstreamToDrake(full_name);
AddModelsFromSdfFile(full_name, package_map, &plant);
plant.Finalize();

Expand Down Expand Up @@ -1961,7 +1952,6 @@ GTEST_TEST(SdfParser, FramesAsJointParentOrChild) {
MultibodyPlant<double> plant(0.0);

PackageMap package_map;
package_map.PopulateUpstreamToDrake(full_name);
AddModelsFromSdfFile(full_name, package_map, &plant);
ASSERT_TRUE(plant.HasModelInstanceNamed("parent_model"));

Expand Down Expand Up @@ -2016,7 +2006,9 @@ GTEST_TEST(SdfParser, InterfaceAPI) {
"drake/multibody/parsing/test/sdf_parser_test/interface_api_test/"
"top.sdf");
PackageMap package_map;
package_map.PopulateUpstreamToDrake(sdf_file_path);
package_map.AddPackageXml(FindResourceOrThrow(
"drake/multibody/parsing/test/sdf_parser_test/interface_api_test/"
"package.xml"));
MultibodyPlant<double> plant(0.0);

DRAKE_ASSERT_NO_THROW(AddModelFromSdfFile(sdf_file_path, "", package_map,
Expand Down Expand Up @@ -2123,7 +2115,6 @@ GTEST_TEST(SdfParser, CollisionFilterGroupParsingTest) {
MultibodyPlant<double> plant(0.0);
SceneGraph<double> scene_graph;
PackageMap package_map;
package_map.PopulateUpstreamToDrake(full_sdf_filename);

// Read in the SDF file.
AddModelFromSdfFile(full_sdf_filename, "", package_map, &plant, &scene_graph);
Expand Down
3 changes: 0 additions & 3 deletions multibody/parsing/test/detail_urdf_geometry_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,6 @@ class UrdfGeometryTests : public testing::Test {
root_dir_ = full_path.substr(0, found);
}

// TODO(sam.creasey) Add support for using an existing package map.
package_map_.PopulateUpstreamToDrake(full_path);

const XMLElement* node = xml_doc_.FirstChildElement("robot");
ASSERT_TRUE(node);

Expand Down
7 changes: 0 additions & 7 deletions multibody/parsing/test/detail_urdf_parser_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ GTEST_TEST(MultibodyPlantUrdfParserTest, DoublePendulum) {
std::string full_name = FindResourceOrThrow(
"drake/multibody/benchmarks/acrobot/double_pendulum.urdf");
PackageMap package_map;
package_map.PopulateUpstreamToDrake(full_name);
AddModelFromUrdfFile(full_name, "", package_map, &plant, &scene_graph);
plant.Finalize();

Expand Down Expand Up @@ -119,7 +118,6 @@ GTEST_TEST(MultibodyPlantUrdfParserTest, TestAtlasMinimalContact) {
std::string full_name = FindResourceOrThrow(
"drake/examples/atlas/urdf/atlas_minimal_contact.urdf");
PackageMap package_map;
package_map.PopulateUpstreamToDrake(full_name);

AddModelFromUrdfFile(full_name, "", package_map, &plant, &scene_graph);
plant.Finalize();
Expand All @@ -140,7 +138,6 @@ GTEST_TEST(MultibodyPlantUrdfParserTest, TestAddWithQuaternionFloatingDof) {
const std::string model_file =
FindResourceOrThrow(resource_dir + "zero_dof_robot.urdf");
PackageMap package_map;
package_map.PopulateUpstreamToDrake(model_file);

MultibodyPlant<double> plant(0.0);
SceneGraph<double> scene_graph;
Expand All @@ -155,7 +152,6 @@ GTEST_TEST(MultibodyPlantUrdfParserTest, TestOptionalSceneGraph) {
const std::string full_name = FindResourceOrThrow(
"drake/examples/atlas/urdf/atlas_minimal_contact.urdf");
PackageMap package_map;
package_map.PopulateUpstreamToDrake(full_name);
int num_visuals_explicit{};
{
// Test explicitly specifying `scene_graph`.
Expand All @@ -182,7 +178,6 @@ GTEST_TEST(MultibodyPlantUrdfParserTest, JointParsingTest) {
"drake/multibody/parsing/test/urdf_parser_test/"
"joint_parsing_test.urdf");
PackageMap package_map;
package_map.PopulateUpstreamToDrake(full_name);

MultibodyPlant<double> plant(0.0);
SceneGraph<double> scene_graph;
Expand Down Expand Up @@ -440,7 +435,6 @@ ::testing::AssertionResult FrameHasShape(geometry::FrameId frame_id,
void TestForParsedGeometry(const char* sdf_name, geometry::Role role) {
const std::string full_name = FindResourceOrThrow(sdf_name);
PackageMap package_map;
package_map.PopulateUpstreamToDrake(full_name);
MultibodyPlant<double> plant(0.0);
SceneGraph<double> scene_graph;
plant.RegisterAsSourceForSceneGraph(&scene_graph);
Expand Down Expand Up @@ -831,7 +825,6 @@ GTEST_TEST(MultibodyPlantUrdfParserTest, CollisionFilterGroupParsingTest) {
"drake/multibody/parsing/test/urdf_parser_test/"
"collision_filter_group_parsing_test.urdf");
PackageMap package_map;
package_map.PopulateUpstreamToDrake(full_name);

MultibodyPlant<double> plant(0.0);
SceneGraph<double> scene_graph;
Expand Down
5 changes: 4 additions & 1 deletion multibody/parsing/test/package_map_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,10 @@ GTEST_TEST(PackageMapTest, TestPopulateMapFromFolderExtraTrailingSlashes) {
VerifyMatchWithTestDataRoot(package_map);
}

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
// Tests that PackageMap can be populated by crawling up a directory tree.
GTEST_TEST(PackageMapTest, TestPopulateUpstreamToDrake) {
GTEST_TEST(PackageMapTest, DeprecatedTestPopulateUpstreamToDrake) {
const string root_path = GetTestDataRoot();
const string sdf_file_name = FindResourceOrThrow(
"drake/multibody/parsing/test/"
Expand All @@ -236,6 +238,7 @@ GTEST_TEST(PackageMapTest, TestPopulateUpstreamToDrake) {
package_map.PopulateUpstreamToDrake(sdf_file_name);
VerifyMatch(package_map, expected_packages);
}
#pragma GCC diagnostic pop

// Tests that PackageMap can be populated from an env var.
GTEST_TEST(PackageMapTest, TestPopulateFromEnvironment) {
Expand Down

0 comments on commit 6aa2b9a

Please sign in to comment.