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

icub-models python library: Raise a FileNotFoundError if the model path is not found #241

Merged
merged 8 commits into from
Nov 6, 2024

Conversation

xela-95
Copy link
Member

@xela-95 xela-95 commented Nov 5, 2024

Instead of returning a placeholder Path("...") from

def get_model_file(robot_name: str) -> pathlib.Path:
model_file = get_models_path() / robot_name / "model.urdf"
if model_file.is_file():
return model_file
else:
return pathlib.Path("...")
when the model name is not found I think it is more predictable and safe to raise an exception here.

@xela-95
Copy link
Member Author

xela-95 commented Nov 5, 2024

CC @traversaro

@traversaro
Copy link
Member

I agree! Let's check what @GiulioRomualdi thinks.

@xela-95
Copy link
Member Author

xela-95 commented Nov 5, 2024

I think we need also to update the action versions here, like the pypi action is failing with:

Error: This request has been automatically failed because it uses a deprecated version of `actions/upload-artifact: v2`. Learn more: https://github.blog/changelog/2024-02-13-deprecation-notice-v1-and-v2-of-the-artifact-actions/

@traversaro
Copy link
Member

I think we need also to update the action versions here, like the pypi action is failing with:

Error: This request has been automatically failed because it uses a deprecated version of `actions/upload-artifact: v2`. Learn more: https://github.blog/changelog/2024-02-13-deprecation-notice-v1-and-v2-of-the-artifact-actions/

Feel free to update it in this PR, thanks!

@traversaro traversaro changed the title Raise a FileNotFoundError if the model path is not found icub-models python library: Raise a FileNotFoundError if the model path is not found Nov 6, 2024
@traversaro
Copy link
Member

(I changed the PR title to be more descriptive as it is going to be included in automatically generated changelog)

@xela-95
Copy link
Member Author

xela-95 commented Nov 6, 2024

I have updated the workflow actions and started using conda and miniforge instead of mamba and mambaforge as suggested in robotology/robotology-superbuild#1653. Now also the CI is passing. Ready for review.

@traversaro
Copy link
Member

Thanks!

@xela-95
Copy link
Member Author

xela-95 commented Nov 6, 2024

@traversaro can you also bump a new minor version after this PR, to have a new conda package ready? Thanks!

@traversaro
Copy link
Member

Sure, I can do the bump directly in this PR. @GiulioRomualdi can you quickly check the change?

@traversaro traversaro merged commit 9ccce64 into robotology:master Nov 6, 2024
21 checks passed
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