MultiDict support for params=, data=, and files=

As discussed in [Add MultiDict Support](https://github.com/kennethreitz/requests/issues/1155), this enables `OrderedMultiDict` support in requests by adding the `OrderedMultiDict` and `MultiDict` structures and using them to marshall the `params=`, `data=`, and `files=` keyword arguments.


Also, since we're using py.test, use `assert foo > bar`.


I've added a failing test to prevent this from being accepted (for now), because I've realized that this PR doesn't meet all the requirements.  (It just got lucky and passed the existing tests)


Hey @dmckeone could you rebase this against current master?


@sigmavirus24 No problem!  I've actually never done a rebase before.  I think it went ok, but let me know if I should do something differently.


Well, looks like the rebase didn't go properly.  `from_key_val_list()` should be returning an `OrderedMultiDict`.  I'm going to review all the changes again.

@sigmavirus24 Since this has gotten a little messy with my screwed up rebase, should I just create a separate pull request and close this one?


> Well, looks like the rebase didn't go properly. from_key_val_list() should be returning an OrderedMultiDict. I'm going to review all the changes again.

Can we make a test that fails in the current situation too. :)


@gazpachoking Yes, definitely.  That was actually my first step.


I'm concerned about the implementation of `.utils.to_key_val_list`. It behaves badly when given non-`dict`-descended mappings such as `CaseInsensitiveDict`, as well as non-str sequences containing items with length != 2.

The implementation as of db9083a:

``` python

def to_key_val_list(value):
    """Take an object and test to see if it can be represented as a
    dictionary. If it can be, return a list of tuples, e.g.,

    ::

        >>> to_key_val_list([('key', 'val')])
        [('key', 'val')]
        >>> to_key_val_list({'key': 'val'})
        [('key', 'val')]
        >>> to_key_val_list('string')
        ValueError: cannot encode objects that are not 2-tuples.
    """
    if value is None:
        return None

    if isinstance(value, (str, bytes, bool, int)):
        raise ValueError('cannot encode objects that are not 2-tuples')

    if isinstance(value, dict):
        value = value.items()
        try:
            # Check for MultiDict first
            value = value.items(multi=True)
        except TypeError:
            value = value.items()

    return list(value)
```

I suggest instead:

``` python
import collections

from .compat import basestring

def to_key_val_list(value):
    if value is None:
        return None

    if isinstance(value, collections.Mapping):
        try:
            # Check for MultiDict first
            items = value.items(multi=True)
        except TypeError:
            items = value.items()
    elif (not isinstance(value, basestring) and
          isinstance(value, collections.Sequence) and
          all(len(x) == 2 for x in value)
        ):
        items = value
    else:
        raise ValueError('Value must be a mapping or sequence of 2-iterables')

    return list(items)
```

And I don't really know why it has to be a list, either. Maybe this is out of scope for this PR, let me know if I should handle it separately.


@cdunklau I also noticed that `CaseInsensitiveDict` wasn't caught at any place that checked `isinstance( ... , dict)`, so I altered all of those checks to use `collections.Mapping` instead.  As for the sequence check, that looks ok to me as well, happy to integrate it in this PR, but for now I haven't included it.


@dmckeone yeah I noticed the same thing somewhere in `.models`... but I haven't looked hard enough at it.


Hmm, we should probably fail fast on strings. I edited my code comment above to do that, along with a typo and some imports


@cdunklau After some more thought about your changes, I'm not sure if it would handle all inputs in the same way as the previous code.  If a `value` was a generator, then `all()` would exhaust it, and the setting of `item` would not work correctly.  Perhaps that is why `list()` is used?  Your checks for `basestring` are good though, so I included those in a new commit.


Good point. I _really_ dislike the explicit type checks for bool and int. And we really don't need the check for string, I suspect. How about something more like this:

``` python
def to_key_val_list(value):
    if value is None:
        return None

    if isinstance(value, collections.Mapping):
        try:
            # Check for MultiDict first
            items = value.items(multi=True)
        except TypeError:
            items = value.items()
    elif isinstance(value, collections.Iterable):
        items = value
    else:
        raise ValueError('Value must be a mapping, or iterable of 2-tuples')

    return list((k, v) for k, v in items)
```


I would think it reasonable to assume that `.items()` was a list, so perhaps something more like:

``` python
def to_key_val_list(value):
    if value is None:
        return None

    if isinstance(value, collections.Mapping):
        try:
            # Check for MultiDict first
            items = value.items(multi=True)
        except TypeError:
            items = value.items()
    elif isinstance(value, collections.Iterable):
        items = [(k, v) for k, v in value]
    else:
        raise ValueError('Value must be a mapping, or iterable of 2-tuples')

    return items
```

I'm guessing you are using the generator in list here to catch the 2-tuples?

EDIT:  I suppose we can use a list comprehension, rather than `list()`


`.items` is a view on py3, so that's a very bad assumption :grin:


I think we should return the list comp regardless, because of the py3 behavior.


@cdunklau Agreed.  Made the commit.


Nitpicking, the docstring's example should be updated, and I think that ValueError could be worded better.


@cdunklau Updated.  I had initially kept the same Exception message on purpose, but since it all needs to be updated, I suppose it is better to update the language.


Shiny, looks good at the surface. For what my uninformed opinion is worth, :+1: 


So, I like this change, I was only wondering about a bit of an api change it creates. From the test:

> s = requests.Session()
> s.params = OrderedMultiDict({'multi': 2})
> r = s.request('GET', url, params={'multi': '1'})
> self.assertEqual(r.request.url, "http://httpbin.org/get?multi=2&multi=1")

This used to result in `http://httpbin.org/get?multi=1`, right? I'm not against the new behavior, just pointing out that it is new. Is there a way to override the session setting for `multi` anymore, rather than just extend it with more values?

This behavior would be a bit surprising if trying to override a particular session header or cookie on a request. In fact, maybe there is a bit more of an issue. With `merge_kwargs` using a `MultiDict`, for headers, I think it would be turned into a `CaseInsensitiveDict` later which means either the value of the session header or request header for a given key would be lost, with no control over which one was used.

(Caveat, I have tested nothing. Just reading the code so far, so I could be wrong.)


Hmm, I don't like that `.update` _adds_ items. Shouldn't it overwrite them, like `__setitem__` does? I know that's not how werkzeug's multidict does it, but I think this one should.

There should definitely be an `.update`-like method that uses `__setitem__` instead of `add`.


Discussed a bit more with @cdunklau on IRC, we are thinking that keys from arguments in a request should still continue to override the same keys (rather than add more of those keys) from the session param.

Still not sure if there should be some way to specifically add to the existing session keys for a given request, maybe if the request argument is explicitly a multidict.

Other opinions are certainly needed, as I'm not sure if I'm considering all use cases. I'm thinking though that the most common would be to want to override a session key rather than add a second same key to a dict.


I agree with @gazpachoking and @cdunklau, overriding already set keys is the way to go. Sadly, that'll need special-case code.


Haven't forgotten about this.  Closing this request because the discussion brought up a number of changes that needed to be made, and it's probably best done in a new pull request.


What was the solution to this? I'm trying to use

```python
import requests

from werkzeug.datastructures import ImmutableMultiDict

params = ImmutableMultiDict([('q1', 'value1'), ('q1', 'value2')])
request = requests.Request(method='GET', url='https://my-url.com' params=params)
session = requests.Session()
prepared_request = request.prepare()

# getting "https://my-url.com?q1=value1",
# but expecting "https://my-url.com?q1=value1"&q1=value2"
print(prepared_request.url)

response = session.send(prepared_request)
```

**Update**:

Solved by removing ImmutableMultiDict. This works:

```python
import requests

params = [('q1', 'value1'), ('q1', 'value2')]
request = requests.Request(method='GET', url='https://my-url.com' params=params)
session = requests.Session()
prepared_request = request.prepare()

# "https://my-url.com?q1=value1"&q1=value2"
print(prepared_request.url)

response = session.send(prepared_request)
```