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.

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

Create parent dir, when serializing Sim(2), if it doesnt exist #252

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

johnwlambert
Copy link
Contributor

Instead of crashing, simply create parent dir of requested absolute filepath.

@@ -156,7 +156,7 @@ def save_as_json(self, save_fpath: _PathLike) -> None:
"""
if not Path(save_fpath).exists():
Copy link
Contributor

@benjaminrwilson benjaminrwilson Jul 10, 2021

Choose a reason for hiding this comment

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

Hi @jlambert,

Thanks for adding this. I notice that this function calls save_json_dict. Do you think that it might be convenient if we move directory creation into that function?

My thought is this: https://github.com/argoai/argoverse-api/blob/9ad7251e863590603c3f6064a45830dabd3a15e2/argoverse/utils/json_utils.py#L32 could be modified as:

json_fpath = Path(json_fpath)
json_fpath.parent.mkdir(parents=True, exist_ok=True)
with json_fpath.open("w") as f:
    json.dump(dictionary, f)

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 on this.

Copy link
Contributor

@tagarwal-argoai tagarwal-argoai left a comment

Choose a reason for hiding this comment

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

Left a small comment.

@@ -156,7 +156,7 @@ def save_as_json(self, save_fpath: _PathLike) -> None:
"""
if not Path(save_fpath).exists():
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 on this.

@benjaminrwilson
Copy link
Contributor

@johnwlambert, is there anything I can do to help with this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants