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

Add Support for Multiple Color Formats in Hallway, Robot, Object and Location class. #307

Merged

Conversation

kumar-sanjeeev
Copy link
Contributor

This update allows the Hallway, Robot, Object and Location class to accept the color parameter in multiple formats:

  • RGB list: [R, G, B]
  • Named color string (e.g., "red", "blue")
  • Hexadecimal string (e.g., "#0000FF")

Unit tests have been modified wherever needed.

Copy link
Owner

@sea-bass sea-bass left a comment

Choose a reason for hiding this comment

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

Looks awesome. Just 1 comment.

pyrobosim/pyrobosim/utils/general.py Outdated Show resolved Hide resolved
@sea-bass
Copy link
Owner

sea-bass commented Dec 7, 2024

Actually one more thing, I think the bulleted list in all the comments needs to be formatted to render properly in the docs.

I think it just has to do with whitespace and indentation. Try building the docs locally?

Screenshot_20241207_070832_Chrome

@kumar-sanjeeev
Copy link
Contributor Author

I tried building docs locally. What do you think about this new rendering ?

image

@sea-bass
Copy link
Owner

sea-bass commented Dec 7, 2024

That's perfect -- thanks!

That holds for all the docstrings of the modified classes in this PR too, of course.

@sea-bass
Copy link
Owner

sea-bass commented Dec 7, 2024

You may also need to update the expected error message in the test

@kumar-sanjeeev
Copy link
Contributor Author

I wanted to highlight one thing. While using the Docker setup you provided for local development, I noticed that the docs directory isn’t mounted inside the container in the compose file. Was this intentional?

I think it would be helpful to mount it as a volume. This way, contributors (like me) can locally install the pyrobosim module in Docker and build the docs to verify if they are generated correctly.

Copy link
Owner

@sea-bass sea-bass left a comment

Choose a reason for hiding this comment

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

Thanks!

I'll take a look at making docs build possible in the docker. Good idea.

@sea-bass sea-bass merged commit 061fa57 into sea-bass:main Dec 7, 2024
6 checks passed
@kumar-sanjeeev kumar-sanjeeev deleted the feat-support-multiple-color-formats branch December 18, 2024 16:46
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.

2 participants