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

Matplot++ skeleton code #39

Merged
merged 5 commits into from
Nov 16, 2023
Merged

Matplot++ skeleton code #39

merged 5 commits into from
Nov 16, 2023

Conversation

Tyler-Lentz
Copy link
Contributor

@Tyler-Lentz Tyler-Lentz commented Oct 22, 2023

Reworking this PR to just include the skeleton code for this functionality since it changes how the polygon and polyline classes are handled, and so we can get the matplot dependency merged in.

Ideally this branch would have been called chore/matplotplusplus because that is more what this is, but there are some code changes so maybe it's a little bit of a feature as well.

Implementation will be in a separate branch

@Samir-Rashid
Copy link
Contributor

What's the status on this? #51 is going to get merged in the next few days, so this should be unblocked after that.

@Tyler-Lentz
Copy link
Contributor Author

Shoutout @AskewParity, the dubins unit tests caught their first error in the code base. I messed up resolving a merge conflict which arose from introducing the color data member to the XYZ coord struct, so the psi value was not being correctly set. Roughly 80% of the dubins tests started failing, so I found it right away.

@Tyler-Lentz Tyler-Lentz changed the title Matplot++ Wrapper Class Matplot++ skeleton code Nov 15, 2023
@Samir-Rashid Samir-Rashid marked this pull request as ready for review November 15, 2023 01:55
Copy link
Member

@atar13 atar13 left a comment

Choose a reason for hiding this comment

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

Looks great. Structure of all the classes make sense. I like the use of std::optional. CMake is clean and works in the container.

Maybe we can add matplot to the playground integration test just so we can always verify that we can still include it.

@Tyler-Lentz Tyler-Lentz merged commit 0cb7f87 into main Nov 16, 2023
1 of 2 checks passed
@Tyler-Lentz Tyler-Lentz deleted the feat/matplot-wrapper branch November 16, 2023 00:38
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