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
Make autocall regexp's configurable. #1713
Make autocall regexp's configurable. #1713
Conversation
I should mention that even with this change I cannot get
|
Could you add tests for one or both of these parts? Thanks. |
Test results for commit d1f107f merged into master
Not available for testing: python2.6 |
Yes, see attached. There might be an easier way to do so, but I ended up creating a new |
Test results for commit 9d723ae merged into master
Not available for testing: python2.6, python3.1 |
Looks good to me, but I'll let someone else have a quick glance over it (ping @fperez, @ellisonbg, @minrk) |
@@ -1411,3 +1411,14 @@ def validate(self, obj, value): | |||
if port >= 0 and port <= 65535: | |||
return value | |||
self.error(obj, value) | |||
|
|||
class CRegExp(TraitType): | |||
"""A trait for a compiled regular expression.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really the right description? If the validation attempts to compile, why is the trait itself called 'compiled'? I'm a little confused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose it should have the word "coerced" in there? That's why there is the C prefix, to be consistent with other traits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess what it needs is a more complete docstring: regexps lingo is often ambiguous, because people often use the word 'regex' to describe either the pattern (which is a plain string) or the compiled regex (which is a very different object from a string). So I'd like the dosctring here to unambigously and explicitly indicate what it refers to, what it validates, what the constructor takes, etc. In other traits we don't need as much info b/c everybody has a reasonable idea of what an Int should be. But I think regexes deserve special consideration to avoid confusion in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This accepts either a compiled expression or a string, but the attribute will always be a compiled regular expression object (re.compile(re.compile(pat))
is allowed). Adding 'casting' to the description would be appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A casting compiled regular expression trait.
Accepts both strings and compiled regular expressions. The resulting
attribute will be a compiled regular expression.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very clear, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, that's perfect, thanks!
Now with the improved docstring. |
Make autocall regexp's configurable. * Creates a new trait type "CRegExp" which casts a string or regular expression into a compiled regular expression. * Makes the autocall regular expressions, which are only used in AutocallChecker, configurable. Closes #81.
…nfigurable Make autocall regexp's configurable. * Creates a new trait type "CRegExp" which casts a string or regular expression into a compiled regular expression. * Makes the autocall regular expressions, which are only used in AutocallChecker, configurable. Closes ipython#81.
As discussed in #81, there is some (small) demand that that the autocall regular expressions be configurable. For example:
The attached pull request: