-
Notifications
You must be signed in to change notification settings - Fork 1
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
Handy functions #5
base: master
Are you sure you want to change the base?
Conversation
It is possible to achieve the same effect using .map(), but this seems more elegant
All the functions are supposed to be tested anyway
@@ -194,7 +213,7 @@ def alt_parser(stream, index): | |||
|
|||
return alt_parser | |||
|
|||
def seq(*parsers): | |||
def seq(*parsers, f=None): |
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.
Not up to me, but I'd prefer f be named seq_fn for consistency with map/bind.
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.
Well, the difference is that you almost certainly wouldn't pass arguments to map
/bind
by name (and most likely you wouldn't even know what they're called), whereas in this case it's keyword-only, so I wanted to use a shorter name. I do not insist though. Let's wait for @jneen to decide.
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 think that this extra keyword parameter would work better as a separate method, perhaps named combine
. I can see situations where it would be useful outside of the context of seq
, such as after many
, or if you want to reuse a parser defined elsewhere, and it would be cleaner this way I think.
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 may not be following; how would it be different from .map()
?
And my present self is not that convinced with "compare before and after" example from my 2.5-year-younger self, so maybe we don't need that feature at all.
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've added a PR for this feature here - python-parsy#3 - which happens to include docs that answer your question. Let me know what you think.
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.
Oh. Yes, that looks good and is indeed cleaner. 👍
How would you use it with .many()
though? (you don't know the arg count; you use .times()
when you do know)
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.
You're right, times()
would be a better example, although occasionally you might use manyy()
followed by some constructor functions that take an arbitrary number of arguments via*args
.
|
||
def times(self, min, max=None): | ||
# max=None means exactly min | ||
# min=max=None means from 0 to infinity | ||
# max can also be float('inf') |
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.
float('inf')
is math.inf
in modern Python; at least this doc should be updated, not sure about the code - do we keep compatibility with earlier Python 3.x versions?
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.
Looks like math.inf
was only added in 3.5, I wouldn't break 3.3 and 3.4 compat for this. Although we could pull it out as a constant at module level for a performance improvement.
@@ -1,4 +1,4 @@ | |||
from parsy import string, regex, generate, ParseError, letter, digit | |||
from parsy import * |
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 introduces tons of flake8 errors/warnings, plus can give static analysers/"goto definition" a harder time for anyone working on the code. I don't think it is worth it.
An implementation of a few ideas found in Parsec:
parser.should_fail()
succeeds (consuming no input) ifparser
failsparser << other.should_fail()
— this is how you do Parsec'snotFollowedBy
(other.should_fail() >> parser).many()
for Parsec'smanyTill
seq(*parsers, f=...)
allows for easier post-processing, compare before and afterparser.sep_by(sep)
is roughly equivalent toparser.times(1) + (sep >> parser).many()
. It can however handle zero-times case and hasmin
andmax
keyword-only arguments.lparen >> (expr.sep_by(comma, min=2) | expr.times(1) << comma | success([])) << rparen