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

change keyPressEvent log level #993

Merged
merged 1 commit into from
Nov 11, 2024
Merged

Conversation

micahwoodard
Copy link
Collaborator

@micahwoodard micahwoodard commented Nov 11, 2024

Pull Request instructions:

  • Please follow the update protocol
  • Answer the questions below in detail. Your responses will be emailed to experimenters.
  • If the experimenters must do anything new, provide detailed step by step instructions on the wiki
  • If computer maintainers need to manually update anything, provide detailed step by step instructions
  • Use markdown syntax in order for your comments to be rendered reliably in the email: "1." instead of "1)", use four spaces for indents.
  • If you use the keyword "skip email" in the title, it will skip the email updates
  • Merges from "develop" into "production_testing" should use the keyword "production merge" in the title for reliable indexing of updates
  • Merges from "production_testing" into "main" should use the keyword "update main"

Describe changes:

  • change log level from error to warning in keyPressEvent function to avoid creating issues when widgets have empty strings instead of floats

What issues or discussions does this update address?

Describe the expected change in behavior from the perspective of the experimenter

  • None

Describe any manual update steps for task computers

  • None

Was this update tested in 446/447?

  • 446/7

@alexpiet
Copy link
Collaborator

I support changing this to a warning so we don't generate tons of errors. However, we should still refactor the underlying code which is a mess. Up to @micahwoodard on the priority for that refactor

@XX-Yin
Copy link
Collaborator

XX-Yin commented Nov 11, 2024

I support changing this to a warning so we don't generate tons of errors. However, we should still refactor the underlying code which is a mess. Up to @micahwoodard on the priority for that refactor

Hey @alexpiet, I understand that the format check in the code is inherently complex, primarily due to the nature of the functions we aim to implement. If you have a better solution in mind, please feel free to share it and implement it!

@micahwoodard
Copy link
Collaborator Author

I think refactoring would work well with this issue. I will merge and close associated issues for now. It would be nice to clarify all the TP_ values and how they are being used to define the task performed. Maybe we can discuss in person

@micahwoodard micahwoodard merged commit d789789 into develop Nov 11, 2024
This was referenced Nov 11, 2024
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