Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add resolve_filepath for url scheme such that package://, model:// #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add resolve_filepath for url scheme such that package://, model:// #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file added urdfpy/ros_utils.py
View file
Open in desktop
Empty file.
20 changes: 20 additions & 0 deletions urdfpy/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
"""Utilities for URDF parsing.
"""
import os
try:
# for python3
from urllib.parse import urlparse
except ImportError:
# for python2
from urlparse import urlparse

from lxml import etree as ET
import numpy as np
Expand Down Expand Up @@ -180,6 +186,19 @@ def unparse_origin(matrix):
return node


def resolve_filepath(base_path, file_path):
parsed_url = urlparse(file_path)
dirname = base_path
file_path = parsed_url.netloc + parsed_url.path
while not dirname == '/':
resolved_filepath = os.path.join(dirname, file_path)
print(resolved_filepath)
if os.path.exists(resolved_filepath):
return resolved_filepath
dirname = os.path.dirname(dirname)
return False
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the tests, this will return false, then in get_filename, os.path.isabs fails because it's trying to evaluate False



def get_filename(base_path, file_path, makedirs=False):
"""Formats a file path correctly for URDF loading.

Expand All @@ -199,6 +218,7 @@ def get_filename(base_path, file_path, makedirs=False):
The resolved filepath -- just the normal ``file_path`` if it was an
absolute path, otherwise that path joined to ``base_path``.
"""
file_path = resolve_filepath(base_path, file_path)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with comment above, I think you need a check to see if resolve_filepath returns False

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
file_path = resolve_filepath(base_path, file_path)
resolved_file_path = resolve_filepath(base_path, file_path)
if resolved_file_path != False:
file_path = resolved_file_path

fn = file_path
if not os.path.isabs(file_path):
fn = os.path.join(base_path, file_path)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when given a relative path, this gives the wrong path. resolve_filepath returns base_path + file_path, then this if makes fn = base_path + base_path + file_path. Should this be os.path.join(os.getcwd(), file_path) instead?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fn = os.path.join(base_path, file_path)
fn = os.path.join(os.getcwd(), file_path)

Expand Down