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 a function to check if TE values are in ms, otherwise update them #1151

Merged
merged 2 commits into from
Nov 22, 2024

Conversation

eurunuela
Copy link
Collaborator

Closes #1150.

Changes proposed in this pull request:

  • Adds a function to check if the TE values given by a user are in ms. If they are not and are below 1, it assumes they are in seconds and they get updated by multiplying by 1000. If the values are a combination of higher than one and lower than one, the function raises an error.
  • Adds a unit test to ensure the functionality is correct.

@eurunuela eurunuela self-assigned this Nov 21, 2024
@eurunuela
Copy link
Collaborator Author

I'm pretty sure this should do it but I think it would be helpful if @handwerkerd tested it on the dataset you showed earlier.

Copy link
Member

@tsalo tsalo left a comment

Choose a reason for hiding this comment

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

I generally would prefer to raise a warning and continue without changing the data, but I can't imagine a scenario where a user might provide a value < 1 and actually intend milliseconds, so I'm good with your approach.

Copy link
Member

@handwerkerd handwerkerd 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 for doing this! Just one minor text change.

Co-authored-by: Dan Handwerker <[email protected]>
@eurunuela eurunuela merged commit dddf251 into ME-ICA:main Nov 22, 2024
14 checks passed
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.

Add sanity check for echo times
3 participants