BUG: allow happi search to match int numerics
#261
Conversation
| client_args[criteria] = value | ||
| continue | ||
| else: | ||
| value = str(float(value)) |
There was a problem hiding this comment.
Couldn't a floating point value be somewhat problematic for regex mode? The above comment and continue gave me pause to think about it a bit:
- Say we passed in "3.14" we might match "3514" or "3x14" just the same.
.would be interpreted as a regex anychar, wouldn't it? - Does float-based value searching also work as a full match or a partial match? That is, if
12345.678is in the database and I search for5.678what happens? (lazy question...)
There was a problem hiding this comment.
These are good points.
- I think you're right, ints will always get an escaped
., but floats will get passed through as is. Is this a problem though? We could argue if someone is searching in regex mode they should be escaping their.if they need to? - The float-based value searching seems to not work with partial matches, though I didn't do that intentionally.
There was a problem hiding this comment.
We could argue if someone is searching in regex mode they should be escaping their . if they need to?
If someone is doing a regex search with a thing that looks like a floating point value... is it a floating point value or a regex?
... I think it's a regex and probably shouldn't go through the float(str(float)) conversion? 🤷
Is this a problem though?
Likely not - just trying to dig into the implications of the changes in my usual excessive way 😁
There was a problem hiding this comment.
I think this is an annoyance with using regex in general to search for floating point numbers rather than a unique problem to happi
Float-based values not partial matching almost seems like a feature rather than a bug...
There was a problem hiding this comment.
That's a good point about the processing of regex, Ken
There was a problem hiding this comment.
Likely not - just trying to dig into the implications of the changes in my usual excessive way 😁
I don't think it's excessive, it's just thorough 😄
Float-based values not partial matching almost seems like a feature rather than a bug...
I wish I could claim credit for that haha
| if float(value) == int(float(value)): | ||
| # value is an int, allow the float version (optional .0) | ||
| logger.debug(f'looking for int value: {value}') | ||
| value = f'^{int(float(value))}(\\.0+$)?$' |
There was a problem hiding this comment.
I think this is good as you have it:
- Requiring it to match the entire integer value - as marked by
^...$ - With or without the trailing
.0.
| # float_result = runner.invoke(happi_cli, ['--path', happi_cfg, | ||
| # 'search', '--names', 'z=3.0']) | ||
| # assert int_result.output == '' | ||
| # assert float_result.output == '' |
There was a problem hiding this comment.
👍 for leaving a test for future use
I think another way to do this is to add the test (as an independent function) and mark it as xfail
|
The screenshot in the description above confused the heck out of me because its "after -> before" instead of "before -> after" but I caught myself before making an equally confused comment. |
|
I'm going to merge this, but another edge case I didn't consider is scientific notation. At this point, we're patching over an interface that really doesn't seem to handle numerics correctly. This is probably worth a deeper look, perhaps at other search methods on the backends. |
Description
Motivation and Context
closes #256
In building tests I realized
happi editdoes not allow editing ofextraneousinformation. A portion of the new test has been commented out for when that is fixed. (see #262)How Has This Been Tested?
Test suite, interactively
Where Has This Been Documented?
This PR