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

Throw error on nonexistent channel reads in mibitiffs #392

Merged
merged 4 commits into from
Feb 25, 2021

Conversation

ackagel
Copy link
Contributor

@ackagel ackagel commented Feb 24, 2021

Purpose

Adds error message to read_mibitiff if a non-existent channel is provided as input. This makes debugging typos/capitalizations in channel names easier to catch.

Copy link
Member

@ngreenwald ngreenwald left a comment

Choose a reason for hiding this comment

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

Looks good, I think we can use verify_in_list for the actual check

# make sure all passed channels were found
if channels is not None:
channel_names = [return_channel[1] for return_channel in return_channels]
found_channels = [channel not in channel_names for channel in channels]
Copy link
Member

Choose a reason for hiding this comment

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

We can do verify_in_list for this right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

totally. I do think it should be an IndexError instead of a ValueError in this case, so I'll wrap it in a try/except block and re-raise the error as an IndexError.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good. Other option would be to add option to specify error type as an argument to verify_in_list. I'm cool with either

Copy link
Member

@ngreenwald ngreenwald left a comment

Choose a reason for hiding this comment

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

Does the

raise IndexError('Passed unknown channels...') from exc

syntax also pass the message from the original exception? If so, then this is fine. If not, then it's going to hide the info telling people which values they have that are bad

@ackagel
Copy link
Contributor Author

ackagel commented Feb 25, 2021

yep, it passes the original message as well, and also notes that the error was caused by that error; basically builds a little traceback.

@ngreenwald ngreenwald merged commit 5270fe4 into master Feb 25, 2021
@ngreenwald ngreenwald deleted the mibitiff-throw-error branch February 25, 2021 03:02
y2kbugger pushed a commit that referenced this pull request Jul 29, 2021
* adds error message for attempts to read non-existant channels in mibitiffs

* corrects style error

* uses verify_in_list in mibitiff read util
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.

2 participants