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

Vision Refactoring + Lane Marker tests #593

Merged
merged 10 commits into from
Jul 9, 2024
Merged

Vision Refactoring + Lane Marker tests #593

merged 10 commits into from
Jul 9, 2024

Conversation

frompotenza
Copy link
Contributor

Tested in the sim, and it still works as before

@antoinedang
Copy link
Contributor

is it possible to split this up into smaller PRs? This is too massive to review properly (i.e. one PR for refactors, a seperate one for tests)

@MrMondrian
Copy link
Contributor

@frompotenza lgtm. I think a good think for readability in vision would be to make all params uppercase so its clear which values are variables or hyper parameters

@frompotenza
Copy link
Contributor Author

is it possible to split this up into smaller PRs? This is too massive to review properly (i.e. one PR for refactors, a seperate one for tests)

almost nothing is for tests, only the stuff in the tests folder

@frompotenza
Copy link
Contributor Author

@frompotenza lgtm. I think a good think for readability in vision would be to make all params uppercase so its clear which values are variables or hyper parameters

Do you mean rosparams?

@antoinedang
Copy link
Contributor

@frompotenza lgtm. I think a good think for readability in vision would be to make all params uppercase so its clear which values are variables or hyper parameters

Do you mean rosparams?

I'd assume so, anything thats not a variable. I agree with that idea too, it makes things easier to read

Other than that I'm good to merge if you've tested and its working as good as without these changes. Please 😢 🙏 I don't want to debug vision functions anymore

@frompotenza frompotenza merged commit 71d5a44 into noetic Jul 9, 2024
2 checks passed
@frompotenza frompotenza deleted the vision-tests branch July 9, 2024 15:03
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