[pytest-dev] marks - proposals for a new api and a path forward

[Bruno Oliveira]

On Tue, Aug 29, 2017 at 4:53 PM Floris Bruynooghe <flub at devork.be> wrote:

> Hello,
>
> RonnyPfannschmidt <opensource at ronnypfannschmidt.de> writes:
> > Am 29.08.2017 um 15:18 schrieb Bruno Oliveira:
> >> On Sun, Aug 27, 2017 at 4:04 AM Ronny Pfannschmidt
> >> <opensource at ronnypfannschmidt.de
> >> <mailto:opensource at ronnypfannschmidt.de>> wrote:
> >>     what i imagine is an api like this:
> >>
> >>     node.find_marks() -> iterates over all marks of all nodes
> >>     node.find_marks('name') -> iterates over all marks that have a name
> >>     node.find_marks(SomeType) -> iterates over all marks that are
> instaces
> >>     of the type
> >>
> >>
> >> This seems reasonable. Minor suggestion, perhaps name the methods
> >> "iter_marks" instead? Seems better based on the descriptions of each one
> >> ("iterates over all marks...")
> >
> > i'd like to bikeshed on the names a bit in order to give them exactly
> > the meaning they should have - perhaps i should split iteration and
> > filtering even
>
> In general I'd agree that splitting the iteration and filtering might be
> more pythonic, also python has generally moved away from calling things
> "iter_*" I think.  Lastly I find the Node.find_marks_with_nodes()
> variation rather clumsy as well.
>
> After thinking about this a bit the best I can come up with is the
> simple Node.marks() method.  As is common in Python it takes no
> arguments and does not filter, it just returns an iterator.  To make
> that work you'd also need to change what object the iterator returns, I
> imagine something like this:
>
> class MarkHolder:
>     name = 'old_mark_name' or None
>     obj = SomeType or whatever obj the old ones are
>     origin = Node
>
> (As an aside I'm guessing this class would be a prime candidate to be
> marked @attr.s thing)
>
> While you get an extra layer it's both rather Pythonic in the API with
> respect to the behaviour of the iterator method as well as py.testy with
> the use of holder.obj.  (I'm not sure if "py.testy" is generally
> perceived as a desirable property or not ;-))
>
>
> >>     node.push_mark(markobj) -> pushes a mark to a node, always requires
> a
> >>     mark object wither taken from pytest.mark or a new style one
>
> This method I do like, it should probably come with a symmetric
> Node.pop_mark() as well which would mirror dict.pop() behaviour.
>
>
> >>     following up the evaluation of skip marksfor example would look like
> >>     this:
> >>
> >>     for mark in node.find_marks('skip'):
> >>        if eval_mark(node, mark):
> >>           pytest.skip(mark.args)
>
> for mark in (m for m in node.marks() if m.name == 'skip'):
>     ...
>
> >>     a more complex marker could be wored the following
> >>
> >>     for orgin, blocker in node.find_marks_with_node(Blocker):
> >>        blocker.maybe_trigger_outcome(orgin=orgin, current=node)
>
> for mark in (m for m in node.marks() if isinstance(m.obj, Blocker)):
>     mark.obj.maybe_trigger_outcome(origin=mark.origin, current=node)
>
> Having written these examples without any filtering API I do admit I'm
> doubting my earlier evaluation about this being a more Pythonic API.  It
> certainly is a lot more writing and probably even harder to read as
> well.  But I thought I'd still send the mail as it's still useful food
> for thought.
>

Not sure, I find the examples quite readable if we name the expressions:

```python
skip_marks = (m for m in node.marks() if m.name == 'skip')
for mark in skip_marks:
    ...
```

```python
blocker_marks = (m for m in node.marks() if isinstance(m.obj, Blocker))
for mark in blocker_marks:
    mark.obj.maybe_trigger_outcome(origin=mark.origin, current=node)
```

So I think the idea is worthwhile. Also, it is simpler to add more ways to
filter the API later than remove something later, so there's that to
consider as well.

But I like that we seem to agree on the general direction of the API.

Cheers,
Bruno.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/pytest-dev/attachments/20170829/b737e2f5/attachment-0001.html>