Skip to content
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

More expressive factor condition expression support #698

Closed
jurko-gospodnetic opened this issue Dec 6, 2017 · 8 comments
Closed

More expressive factor condition expression support #698

jurko-gospodnetic opened this issue Dec 6, 2017 · 8 comments
Assignees
Labels
area:configuration feature:new something does not exist yet, but should level:medium rought estimate that this might be neither easy nor hard to implement

Comments

@jurko-gospodnetic
Copy link

jurko-gospodnetic commented Dec 6, 2017

We currently support the following operators in tox's factor condition expressions:

but this support can be updated to be more expressive by supporting (/) parenthesis for grouping.

This would not allow any additional meaning that can not already be expressed using currently available support, but it would help by allowing us to make complex expressions shorter and possibly closer to the way developer has the logic envisioned in their mind.

It would also possibly help clean up the code by allowing us to extract the factor condition parsing into a separate grammar parsing module instead of being just kinda quick-fixed into the config.py module.

@gaborbernat
Copy link
Member

@jurko-gospodnetic can you come up with a few examples where this would help?

@jurko-gospodnetic
Copy link
Author

jurko-gospodnetic commented Dec 6, 2017

Simple example shortening a conditional expression could be:

py27-dev, py33-dev, py36-dev: something

which could be shortened to just:

(py27,py33,py36)-dev: something

showing us how this effectively provides similar expansion support as we have when defining an envlist.

This makes the code more maintainable as for instance, if you want to add factor django there, you now just add it in one place instead of three.

Another interesting use case is based on the fact that !(a,b,c) is equivalent to !a-!b-!c and !(a-b-c) is equivalent to !a,!b,!c and which one of those is simpler for a developer to grasp changes depending on what those factors are modeling in the developer's mind.

But again, parenthesis do not bring new features to the table, same as in boolean expressions where and, or and not operators are enough to form any boolean statement, but using parenthesis allows for some expression transformations that can make it shorter, less duplicated or easier to grasp, thus easier to maintain.

@gaborbernat gaborbernat added area:configuration feature:new something does not exist yet, but should level:medium rought estimate that this might be neither easy nor hard to implement labels Dec 6, 2017
@gaborbernat
Copy link
Member

I think it's syntatic sugar, but from a configuration point of view is new feature. Feel free to go ahead with a PR for it. I suggest making the config.py a module (config, what it's in config.py move it into the __init__.py, and then feel free to extract the factorial handling into a factorials.py.

@obestwalter
Copy link
Member

obestwalter commented Jan 26, 2018

Personally I am not a big fan of extending the existing configuration format even more. A lot of bugs and feature requests have to do with the shortcomings of the existing format and weaknesses in the homegrown parsing of it, so I would suggest slowly freezing the existing format and declare it feature complete and only fix bugs for it and see if we can get the advanced expressiveness that is wished for by a (in my estimation) small subset of users by introducing a new configuration format that is more powerfull and malleable.

@vlcinsky
Copy link

@jurko-gospodnetic The new proposed syntax sounds practical as it allows in many cases using shorter and more readable expressions.

I understand the @obestwalter reservation but the proposal sounds relatively conservative (just simplify syntax without adding new logical operations). If the proposal:

  • separates parsing into it's own unit of code
  • takes care of good test suite checking existing as well as new format

it may contribute to both expression readability well as to fixing some older issues related to these expressions.

@RonnyPfannschmidt
Copy link

creating a hypothesis model that captures the desired behaviours may be able to help sorting out the issues in more detail (as traditional testing is simply not up to the task)

@obestwalter
Copy link
Member

obestwalter commented Jan 7, 2019

Proposing to close as wontfix and referring this (or something along those lines) to be implemented in #999.

@gaborbernat
Copy link
Member

Agree with @obestwalter.

@tox-dev tox-dev locked and limited conversation to collaborators Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area:configuration feature:new something does not exist yet, but should level:medium rought estimate that this might be neither easy nor hard to implement
Projects
None yet
Development

No branches or pull requests

5 participants