Replace ParamList with Group

[veyndan]

Summary of changes

No functional changes. I've wanted to better understand the parsing of SPARQL, and have found that ParamList makes the parsing unnecessarily more complex. Replacing ParamList with the built-in Group operator simplifies the ability to understand how a SPARQL query is being parsed.

In order to replace ParamList with Group, I just did a find and replace of ParamList(foo) with Group(Param(foo)). Perhaps replacing instances of ZeroOrMore(ParamList(foo)) should be replaced with Group(ZeroOrMore(Param(foo)) instead of ZeroOrMore(Group(Param(foo)), but I'm not too sure of the benefits of one over the other, so I instead opted for what semantically matches what we had prior to this change.

Checklist

• Checked that there aren't other open pull requests for the same change.
• Checked that all tests and type checking passes.
• Considered granting push permissions to the PR branch, so maintainers can fix minor issues and keep your PR up to date.

[coveralls]

Coverage remained the same at 90.631% when pulling 65f5d4c on veyndan:rm-ParamList into 6eec3bd on RDFLib:main.

[aucampia]

Could it make sense to raise an exception if this condition does not match?

[veyndan]

Yeah I think that's a lot clearer, as otherwise it looks like we're filtering our for ParamValues, but they should all be ParamValues. Fixed in latest commit.

[ptmcg]

Raising a TypeError here conflicts with this line in the docstring: "Any sub-tokens that are not Params will be ignored." So raising TypeError here may start breaking some existing code.

[veyndan]

I completely missed that! I'm not sure how true that line in the docstring is, but I think it's worth an investigation in a separate PR.

[aucampia]

@veyndan thanks for the PR and apologies for the late review. It looks correct to me but I'm going to try get someone with some knowledge of pyparsing to also weigh in and do another review after thinking about it a bit, but I think this approach is significantly better than using a custom class.

[veyndan]

@aucampia Is this okay to merge?

[ptmcg]

I have a few more comments to make on the use of Group, which I'll try to write up this evening.

[ptmcg]

It has been a hairy few weeks, I'm sorry this took so long.

I've started a new page in the pyparsing wiki for pyparsing design topics, with the first topic about using Group. Please look it over (comments and suggestions welcome!), and if you like, we can discuss in more detail how this relates to ParamList in your parser.

[aucampia]

Thanks @ptmcg , I will have a look.

@veyndan sorry I have been a bit busy, will try look again this week and process the inputs from @ptmcg