Added ListParameter and TupleParameter.

[dlstadther]

Dynamic requires serializes all Parameter. The default serialize(...) method casts everything to strings. Lists and tuples need to remain lists and tuples when serialized and parsed.

This is added as a solution to Issue #1607

[erikbern]

Great!

Is it easy for you to add your previously breaking example as a unit test?

[dlstadther]

I've made my (first) attempt at writing a python test. I used the enum tests as a basis. If it sucks, I'll try again.

[erikbern]

I think this should say
self.assertEqual(FooList.a_list, a.parse(a.serialize(b_list)))

[dlstadther]

My bad. That's what I intended, but clearly not what I wrote. Fixed.

[erikbern]

LGTM except the comments above.

is it easy for you to add the test with dynamic requirements too? (not blocking)

[davemt]

I think you are trying to test that when you take a python value and ser-de it it ends up at the same python value, no? So you want something like

assertEqual(b_list, a.parse(a.serialize(b_list))

[davemt]

Also have you tried running tests locally? It helps speed up the test dev process. You can even run just specific tests from just one module, see the CONTRIBUTING doc for tox commands

[dlstadther]

Yes, that is what i'm trying to test. Writing tests is still new to me. Thanks! I'll make that correction.

[dlstadther]

I have some issues here. I'll resume work on it tomorrow.

[Tarrasch]

Can you add docs? In particular how do you specify elements on CLI/config file. I for one don't know how output of json.loads([1,2]) looks like.

[dlstadther]

Sure can.

json.dumps([1,2]) would return '[1,2]'

json.loads('[1,2]') would return [1,2]

[Tarrasch]

Yea. This is the correct way to resurrect the functionality that got lost from #1133. Looks promising!

[dlstadther]

Can anyone explain to me why Luigi always prints lists as tuples when accessing the value of the parameter directly?

class ListFoo(luigi.Task):
    my_list = luigi.ListParameter()

    def run(self):
        pass

class ParameterTest(LuigiTestCase):
    def test_list(self):
        a = luigi.ListParameter()
        b_list = [1, 2, 3]
        c = ListFoo(my_list=b_list)
        self.assertEqual(c.my_list, a.parse(a.serialize(b_list)))

Returns:

AssertionError: (1, 2, 3) != [1, 2, 3]

[erikbern]

probably because json doesn't have a tuple type so it converts it into a list instead

[erikbern]

you need to cast it back to a tuple

[dlstadther]

I'm confused why i should cast a ListParameter to a tuple

[dlstadther]

I believe all i have left now is testing 1) cli usage and 2) dynamic requires. I'll recommit something by the end of my work day today.

[Tarrasch]

I'm pretty sure it's book-locations right? (hyphen not dash)

[dlstadther]

Is this a luigi/python style choice that I'm not aware?

[dlstadther]

@Tarrasch any explanation of this?

[Tarrasch]

Sorry for asking for even more tests, but this is a pretty core functionality. The docs look great by the way! :)

[dlstadther]

Cli tests keep failing locally, but error message isn't helpful. Checking if travis-ci will throw the same error.

[erikbern]

lgtm

[dlstadther]

If @Tarrasch approves, are we good to merge?