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

15 make the tests work #40

Merged
merged 13 commits into from
Mar 21, 2024
Merged

Conversation

bbean23
Copy link
Collaborator

@bbean23 bbean23 commented Mar 20, 2024

closes #15

  • most of changed are help ensure that the tests pass more reliably
  • the changes to requirements.txt are targeting scipy, to ensure that test_photogrammetry.py passes
  • the changes to sensitive_strings.py let me update the sensitive_strings_allowed_binary_files.csv more easily to keep up with the reorganization of sofast

@bbean23 bbean23 added bug Something isn't working enhancement New feature or request labels Mar 20, 2024
@bbean23 bbean23 requested review from e10harvey and braden6521 March 20, 2024 18:43
@bbean23 bbean23 self-assigned this Mar 20, 2024
@e10harvey e10harvey mentioned this pull request Mar 20, 2024
requirements.txt Outdated Show resolved Hide resolved
Copy link
Collaborator

@e10harvey e10harvey left a comment

Choose a reason for hiding this comment

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

Thank you, @bbean23! In general everything looks great! Braden and I will be working on the ubi8 test failures tomorrow. Other than the pypylon version change, I think this can be merged as long as no new test failures are introduced from the dependency version changes in requirements.txt.

@e10harvey
Copy link
Collaborator

Thank you, @bbean23! In general everything looks great! Braden and I will be working on the ubi8 test failures tomorrow. Other than the pypylon version change, I think this can be merged as long as no new test failures are introduced from the dependency version changes in requirements.txt.

ubi8 camera calibration errors fixed via: #42.

… add warning when pypylon version is below the recommended version
Copy link
Collaborator

@braden6521 braden6521 left a comment

Choose a reason for hiding this comment

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

All looks good to me, Ben!

Copy link
Collaborator

@e10harvey e10harvey left a comment

Choose a reason for hiding this comment

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

It looks like there are also some features added to sensitive strings in this PR.

Going forward, I would prefer that features be separated into a different PR from the test fixes.

@bbean23
Copy link
Collaborator Author

bbean23 commented Mar 21, 2024

Going forward, I would prefer that features be separated into a different PR from the test fixes.

Yeah, I really should have split that out into another ticket, sorry.

Thanks for the review!

@bbean23 bbean23 merged commit 5866250 into sandialabs:develop Mar 21, 2024
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants