#development
- b
billowy-beach-68498
05/09/2016, 6:24 PMwhat is your filemap_integration test py file look like? - b
billowy-beach-68498
05/09/2016, 6:25 PMwith all the tests? - w
wide-energy-11069
05/09/2016, 6:25 PMi applied your patch bascially - w
wide-energy-11069
05/09/2016, 6:25 PMdid you encounter any error? - b
billowy-beach-68498
05/09/2016, 6:26 PMI didn’t try this, what I did was pass a globs object to maybe_list and I saw exception, also I checked the source code and found that exception made sense. - b
billowy-beach-68498
05/09/2016, 6:26 PMlet me try what you did - b
billowy-beach-68498
05/09/2016, 6:35 PMthey all failed with exceptions: Value must be of type (<type 'str'>, <type 'unicode'>) or iterable of type (<type 'str'>, <type 'unicode'>) - w
wide-energy-11069
05/09/2016, 6:37 PMdo you run this as well?./pants test tests/python/pants_test/engine/exp/legacy:filemap_integration - b
billowy-beach-68498
05/09/2016, 6:37 PMyeah that is the command I ran - w
wide-energy-11069
05/09/2016, 6:37 PMalso there’s room to specifyexpected_type - w
wide-energy-11069
05/09/2016, 6:37 PMmaybe_list(value, expected_type=Compatibility.string, raise_type=ValueError): - b
billowy-beach-68498
05/09/2016, 6:38 PMexpected_type takes only 1 type, while in my case, 2 types are expected - b
billowy-beach-68498
05/09/2016, 6:38 PMI think this is a pretty specialized use case - w
wide-energy-11069
05/09/2016, 6:40 PMyou are calling
as well right?if isinstance(element, string_types): - w
wide-energy-11069
05/09/2016, 6:40 PMso
can be a tuple as well?expected_type - w
wide-energy-11069
05/09/2016, 6:40 PMnot as obvious as the interface suggests, but essentially they are all using
methodisinstance - r
rough-minister-58256
05/09/2016, 6:42 PMyeah isinstance can take either a single item or an iterable of types - b
billowy-beach-68498
05/09/2016, 6:42 PMthe difference is, in my case, if type is string, I convert it to a list, otherwise I don’t modify it. while in maybe_list, it expects type and type in list to match each other - b
billowy-beach-68498
05/09/2016, 6:43 PMoh, really? - r
rough-minister-58256
05/09/2016, 6:44 PMCopy codeisinstance(object, class-or-type-or-tuple) -> bool - m
mammoth-pharmacist-54215
05/09/2016, 6:44 PM - b
bored-art-40741
05/09/2016, 6:46 PMIndeed, it can take an arbitrarily nested tuple of types - b
billowy-beach-68498
05/09/2016, 6:48 PMok, in that case I think I can use maybe_list. - b
billowy-beach-68498
05/09/2016, 6:49 PMone more thing, I remember in my last review, Timur made a point that we are trying not to use twitter.common.collections in pants any more, is it still true? - b
bored-art-40741
05/09/2016, 6:49 PMYes - b
bored-art-40741
05/09/2016, 6:49 PMWe have a standing policy to sever all dependency ties with twitter.commons - b
bored-art-40741
05/09/2016, 6:50 PMI think collections is our largest existing dep though, because ofOrderedSet - b
billowy-beach-68498
05/09/2016, 6:50 PMok, in that case maybe_list should not be used then? - b
bored-art-40741
05/09/2016, 6:51 PMDoes that come from
? I figured we'd have inlined it by now if it didt.c - b
bored-art-40741
05/09/2016, 6:51 PMLooks like it does