# Make the glob in --show config=xxx case insensitive if "xxx" is all lower case

XMLWordPrintable

#### Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Story Points:
0.25
• Sprint:
Science Pipelines DM-W16-2, Science Pipelines DM-W16-3
• Team:
Data Release Production

#### Description

See RFC-108

The only difference is that I plan to make --show config=xxx be case-insensitive only if the pattern xxx is all lower-case. If people really want to match "background" lower-case only, they can say --show config=xxx:NOIGNORECASE

#### Activity

Hide
Robert Lupton added a comment -

Could you take a look at this? It's pretty short.

 Branch tickets/DM-4217 set up to track remote branch tickets/DM-4217 from origin. $git diff --stat master   python/lsst/pipe/base/argumentParser.py | 12 ++++++++++--  tests/testArgumentParser.py | 12 ++++++++++++  2 files changed, 22 insertions(+), 2 deletions(-)  Show Robert Lupton added a comment - Could you take a look at this? It's pretty short. Branch tickets/DM-4217 set up to track remote branch tickets/DM-4217 from origin.$ git diff --stat master python/lsst/pipe/base/argumentParser.py | 12 ++++++++++-- tests/testArgumentParser.py | 12 ++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-)
Hide
Russell Owen added a comment -

I am strongly in favor of case-blind searching, so on the whole I am very happy with this change.

However, I dislike treating all-lowercase typing specially, as it seems too magic. And the alternative of :NOIGNORECASE is awfully clumsy, hard to type and hard to remember, though I do understand where you got it from.

My suggestion is to NOT support case-sensitive searching at all. Period. Surely any sane search will only turn up a few matches even if case-blind?

However, if you must support case-sensitive searching, then I would request a shorter and more obvious way to specify it. How about a suffix of ":case" (preferably ignoring case)?

Aside from that, this looks fine to me.

Show
Russell Owen added a comment - I am strongly in favor of case-blind searching, so on the whole I am very happy with this change. However, I dislike treating all-lowercase typing specially, as it seems too magic. And the alternative of :NOIGNORECASE is awfully clumsy, hard to type and hard to remember, though I do understand where you got it from. My suggestion is to NOT support case-sensitive searching at all. Period. Surely any sane search will only turn up a few matches even if case-blind? However, if you must support case-sensitive searching, then I would request a shorter and more obvious way to specify it. How about a suffix of ":case" (preferably ignoring case)? Aside from that, this looks fine to me.
Hide
Robert Lupton added a comment -

I made the change as if someone types --show config=CaseSensitiveString I think they should get what they typed. I don't expect anyone to ever want to do the case-sensitive lower case one, but you cannot just pipe to grep because of the docstrings, so I think we do need a mechanism. As it'll be very rare I think the python-inspired NOIGNORECASE is OK, but I can ask for opinions on c.l.o if you'd like.

Show
Robert Lupton added a comment - I made the change as if someone types --show config= CaseSensitiveString I think they should get what they typed. I don't expect anyone to ever want to do the case-sensitive lower case one, but you cannot just pipe to grep because of the docstrings, so I think we do need a mechanism. As it'll be very rare I think the python-inspired NOIGNORECASE is OK, but I can ask for opinions on c.l.o if you'd like.
Hide
Russell Owen added a comment -

Show
Hide
Russell Owen added a comment -
Show
Russell Owen added a comment - For the record, @rhl started this topic: https://community.lsst.org/t/should-we-make-all-show-config-comparisons-be-case-insensitive/344
Hide
Russell Owen added a comment -

One more minor comment: this

  mat = re.search(r"(.*):NOIGNORECASE$", pattern)  is more simply expressed using endswith. The following is an exact replacement, given the way mat is used later in the code, but there are cleaner ways to do this:   mat = pattern.endswith(":NOIGNORECASE")  Show Russell Owen added a comment - One more minor comment: this mat = re.search(r"(.*):NOIGNORECASE$", pattern) is more simply expressed using endswith . The following is an exact replacement, given the way mat is used later in the code, but there are cleaner ways to do this: mat = pattern.endswith(":NOIGNORECASE")
Hide
Robert Lupton added a comment -

Merged to master

Show
Robert Lupton added a comment - Merged to master

#### People

Assignee:
Robert Lupton
Reporter:
Robert Lupton
Reviewers:
Russell Owen
Watchers:
Robert Lupton, Russell Owen