DM-41764: Improve filter name parsing #45
Conversation
| ] | ||
|
|
||
| EMPTY_FILTERNAMES = [ | ||
| "blank", "empty", "unknown" |
There was a problem hiding this comment.
I don't think we can/should assume that unknown means empty - that's usually a sign that something is wrong in the header service. I think we need to treat that as genuinely not knowing.
|
|
||
| Returns | ||
| ------- | ||
| `dict` |
There was a problem hiding this comment.
I think this line needs to read something like
wheelConfiguration : `dict`
so that it has a variable name, not just a type.
| """ | ||
| out = parseFilterName(filterName) | ||
| if len(out['filter']) > 1 or len(out['disperser']) > 1: | ||
| raise ValueError(f"Got more than one filter or disperser in {filterName}") |
There was a problem hiding this comment.
Maybe include the whole {out} in the message for easier debug.
| filter, disperser : `tuple` of `str` | ||
| The filter and the disperser names. | ||
| """ | ||
| out = parseFilterName(filterName) |
There was a problem hiding this comment.
I don't love out as a variable name here (it's fine internal to the parsing function though, of course, as that's the return value there) but that's not what you're returning here, it's what you're catching and using, so maybe wheelConfig or something?
No description provided.