[WIP/RFC] parametrize: support dicts #5850

blueyed · 2019-09-15T18:20:00Z

TODO:

link to issue where this was discussed (Add support for dictionary argvalues for parametrize #5487 (comment))
doc

TODO: - [ ] link to issue where this was discussed - [ ] doc

RonnyPfannschmidt

While I'm personally oppose to this expression mechanism, it's still of great value to way too many people

we should be careful about the semantics

src/_pytest/mark/structures.py

RonnyPfannschmidt · 2019-09-15T19:04:08Z

testing/python/metafunc.py

+
+            @pytest.mark.parametrize('arg1,arg2,expected_id', {
+                "myid_12": [1, 2, "[myid_12]"],
+                "ignored_id": pytest.param(3, 4, "[myid_34]", id="myid_34"),


We should always warn and use the dict imposed id

Is a warning really useful/necessary?

This will help in situations with mixed aliasing

The warning cam be skipped if the id is equal or none

We should provide a utility method to drop the id explicitly if necessary

This will help in situations with mixed aliasing

Can you elaborate, please?

I think the typical use case is not to use pytest.param here in the first place anyway.

The warning cam be skipped if the id is equal or none

Sure, but still I think people might actually be more annoyed by the warning then.

Which I think leads to you thinking about:

We should provide a utility method to drop the id explicitly if necessary

How/where would that be used then? Isn't _replace good enough?

aliasing -> when reusing predeclared param instances that already have a id, potentially a different one

the warning is absolutely necessary to have an api you cant misuse/misunderstand

a internal method generated by a implementation detail is never enough

aliasing -> when reusing predeclared param instances that already have a id, potentially a different one

Do you have a real-world use case / scenario for this?

the warning is absolutely necessary to have an api you cant misuse/misunderstand

Well, ok - requires to unroll the loop again though (which is ok).
What should be used here warnings or pytest.warn?

We should provide a utility method to drop the id explicitly if necessary

How/where would that be used then? Isn't _replace good enough?

a internal method generated by a implementation detail is never enough

You mean _replace is not enough? (it is documented for named tuples, but you mean that us using NamedTuple is the detail?!)

However, how should the method work? Where is the id dropped? (currently it gets replaced (i.e. dropped) always (from the existing paramset))
Do you mean there should be a way to keep it? (i.e. not use the one from the dict)

asottile · 2019-09-17T06:15:42Z

I don't see why this is necessary when we already have pytest.param(..., id=...) as such I'm -1 until convinced otherwise

asottile · 2019-09-17T06:16:16Z

especially since dictionary order is still undefined in at least one of the versions we support (python3.5, provisional but dependable in python3.6)

blueyed · 2019-09-17T08:41:00Z

@asottile #5487 (comment)

dictionary order is still undefined in at least one of the versions we support (python3.5, provisional but dependable in python3.6)

Well, that should be ok in most cases, and is certainly in the responsibility of the user then.
Should be noted with docs however.

The-Compiler · 2019-09-17T10:07:46Z

FWIW I'm -1 on this as well, because it's IMHO not intuitive that those are test IDs. I first thought this would make it possible to do {"arg1": "value1", "arg2": "value2"}. I think the whole syntax is too ambiguous here.

blueyed · 2019-09-17T10:25:54Z

@The-Compiler

I first thought this would make it possible to do {"arg1": "value1", "arg2": "value2"}.

So a dict with arnames and keys, and their values in a list? (where you have a single one now)
Does not seem to be useful, as values are then in different lists.
(asking just out of curiosity to see why you might have thought that way)

I think the whole syntax is too ambiguous here.

Keep in mind that this is meant for advanced usage only, where you like to have custom ids.
And there it is meant to remove the boilerplate that is necessary currently.

I think it's a nice way of mapping the id to the used values. How could this be expressed better after all then with a dict?

nicoddemus · 2019-09-17T23:04:08Z

TBH I'm also 👎 on the idea... it is not really obvious at first glance what it means. We should probably strive to get away from builtin structures unless when they are absolutely straightforward. pytest.param is extensible by definition (you can easily add new attributes without breaking existing cases), but you can't add extra parameters to the dict definition easily.

Another point is that we already have two methods: the traiditional (argnames, argvalues), and the new pytest.param. I wouldn't like to add a third unless it is a clear win.

blueyed · 2019-09-18T02:04:47Z

Ok - will keep this in my local "fork" then also I guess.

Do you all use pytest.param then really for longer lists, or do you just not care about descriptive ids for longer values (e.g. HTML)?
(I think managing a separate list of ids is not very maintainable also, is it?)

For a workaround: you can use dicts already indirectly (#5487 (comment)) - this is what I was doing before, but is rather verbose/indirect then.

blueyed · 2019-10-21T07:09:10Z

Inlcuded in https://github.com/blueyed/pytest-enhancements.

pytest-dev#5850

Rejected in pytest-dev#5850.

aldanor · 2019-11-26T11:59:30Z

Bit late to the party, but 👍 on the suggestion, sad it'd been rejected... Given general ergonomics of pytest and without scanning the docs in much detail one might expect that something like this should "just work" - because:

test ids are unique
each id uniquely identifies test params
dicts are ordered since 3.6 (and if you're on 3.5 just don't use this feature)

So we have a collection of params each uniquely identified by a unique id, what's the ideal data structure?... There just happens to be one already built-in.

As noted above, it may not be the most extensible way of doing it and may indeed lead to ambiguity in certain edge cases, but I believe it would've improved ergonomics in the majority of typical cases where you have to assign tons of custom ids, as currently you have a choice of either:

Matching a long list of argvalues with a long list of ids manually - this is the worst way, but if you glance at the docs, pytest.param is not mentioned in mark.parametrize docs, so one might think this is the way to go
Retyping pytest.param a million of times, specifically in parts of the code where nothing else makes sense but a pytest.param. Why? Just because.

[WIP/RFC] parametrize: support dicts

e1c8bbf

TODO: - [ ] link to issue where this was discussed - [ ] doc

RonnyPfannschmidt requested changes Sep 15, 2019

View reviewed changes

blueyed added 2 commits September 16, 2019 16:58

fixup! [WIP/RFC] parametrize: support dicts

fbda658

handle single string arg

c1204e3

blueyed added the type: enhancement new feature or API change, should be merged into features branch label Sep 17, 2019

blueyed closed this Sep 18, 2019

blueyed added a commit to blueyed/pytest that referenced this pull request Oct 22, 2019

Merge branch 'param-dict' into mm-param-dict

0197b85

pytest-dev#5850

blueyed added a commit to blueyed/pytest that referenced this pull request Oct 22, 2019

parametrize: support dicts

571c414

Rejected in pytest-dev#5850.

blueyed mentioned this pull request Oct 22, 2019

parametrize: support dicts blueyed/pytest#4

Merged

blueyed added a commit to blueyed/pytest that referenced this pull request Oct 22, 2019

parametrize: support dicts (#4)

ea497ea

Rejected in pytest-dev#5850.

blueyed deleted the param-dict branch November 8, 2019 04:48

blueyed mentioned this pull request Nov 12, 2019

Allow parametrize ids to accept generators #759

Closed

This comment has been minimized.

Sign in to view

The-Compiler mentioned this pull request Jul 29, 2020

Allow list of dictionaries for @pytest.mark.parametrize #7568

Closed

graingert mentioned this pull request Jun 7, 2022

fluent writing style for pytest.param objects when id is wanted #10024

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP/RFC] parametrize: support dicts #5850

[WIP/RFC] parametrize: support dicts #5850

blueyed commented Sep 15, 2019 •

edited

Loading

RonnyPfannschmidt left a comment

RonnyPfannschmidt Sep 15, 2019

blueyed Sep 16, 2019

RonnyPfannschmidt Sep 16, 2019

blueyed Sep 16, 2019

RonnyPfannschmidt Sep 17, 2019

blueyed Sep 17, 2019

asottile commented Sep 17, 2019

asottile commented Sep 17, 2019

blueyed commented Sep 17, 2019

The-Compiler commented Sep 17, 2019

blueyed commented Sep 17, 2019

nicoddemus commented Sep 17, 2019

blueyed commented Sep 18, 2019

blueyed commented Oct 21, 2019

aldanor commented Nov 26, 2019

This comment has been minimized.

This comment has been minimized.

[WIP/RFC] parametrize: support dicts #5850

[WIP/RFC] parametrize: support dicts #5850

Conversation

blueyed commented Sep 15, 2019 • edited Loading

RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

RonnyPfannschmidt Sep 15, 2019

Choose a reason for hiding this comment

blueyed Sep 16, 2019

Choose a reason for hiding this comment

RonnyPfannschmidt Sep 16, 2019

Choose a reason for hiding this comment

blueyed Sep 16, 2019

Choose a reason for hiding this comment

RonnyPfannschmidt Sep 17, 2019

Choose a reason for hiding this comment

blueyed Sep 17, 2019

Choose a reason for hiding this comment

asottile commented Sep 17, 2019

asottile commented Sep 17, 2019

blueyed commented Sep 17, 2019

The-Compiler commented Sep 17, 2019

blueyed commented Sep 17, 2019

nicoddemus commented Sep 17, 2019

blueyed commented Sep 18, 2019

blueyed commented Oct 21, 2019

aldanor commented Nov 26, 2019

This comment has been minimized.

This comment has been minimized.

blueyed commented Sep 15, 2019 •

edited

Loading