Should None values in data be handled differently?

Currently, if data contains `None` values they are passed on to `urllib.urlencode` as normal. The behaviour of `urllib.urlencode` is to always call `str()` on the key/value.

This results in the following:

``` python
urllib.urlencode({'key': None})
# 'key=None'
```

To me this seems like odd behaviour. If anything it would make sense for it to result in `'key='`.

One option would be to do something like `v = v or ''` before encoding the data.

Alternatively exceptions could be raised if a key or value is `None`.

Does this sound like something that should be changed?


Good idea but we should check for `None` instead of `v = v or ''` to support falsy values like `{'key': 0}`.


@johtso Strange thing. I got notification about your post which begins with _Ah yes, of course, forgot about 0. What about False? if (v is None) or (v is False): v =_ but I don't see your post from notification...


Ah, I deleted it while reconsidering the problem :) Sorry for the confusion..

Translating `None` to `''` seems to make sense, but `False` doesn't really have any obvious translation..


@kennethreitz, what do you think? :)


This should not be happening ;)


Maybe keys with None value should not be present in the request at all?
As to True/False I think mapping them to 1/0 sounds reasonable.
This functionality could (should?) be implemented as a hook.


Looks like I've misunderstood things a bit.

`'headers', 'cookies', 'auth', 'timeout', 'proxies', 'hooks', 'params', 'config'` all have keys with `None` values removed from them.

`data` is the only kwarg that _doesn't_ have keys with `None` values removed from it, due to no merging going on. Hence seeing `urllib.urlencode` translating them to `key=None`.

So the only question is, do we want to remove keys with values of `None` from `data` as we do elsewhere, leave the current behaviour (`str(None)`), or replace `None` with `''`.


The only purpose of removal upon `None` is because of kwarg merging when sessions are involved. This only matters for `params`. 

I'd like to not make any changes to the behavior of `urlencode` of possible. No surprises that way :)


Yeah, I think that's probably wise. If you want keys without values in your data, convert `None`s to `''` before passing them to your request.


> `data` is the only kwarg that doesn't have keys with `None` values removed from it, due to no merging going on.

If this is the case then I think it's wrong. The behavior should be consistent.


I agree with @piotr-dobrogost, keys with `None` values should be removed like the other maps.


> The only purpose of removal upon None is because of kwarg merging when sessions are involved. 

Well, regardless of the reason it's convenient and logical.

> This only matters for params. 

As this issue shows this matters not only for params. Current behavior of having  `key=None` as a payload of request does not make sense.


I vote that this should behave how every other browser behaves, which is to just send across an empty string. (Just spent an hour debugging this issue with James Cleary).

Cal Leeming
Simplicity Media Ltd


FYI - to anyone else who is having this issue, we hotfix'd it using the following:

```

    @staticmethod
    def _encode_params(data):
        """Encode parameters in a piece of data.

        If the data supplied is a dictionary, encodes each parameter in it, and
        returns a list of tuples containing the encoded parameters, and a urlencoded
        version of that.

        Otherwise, assumes the data is already encoded appropriately, and
        returns it twice.
        """

        if hasattr(data, '__iter__') and not isinstance(data, str):
            data = dict(data)


        if hasattr(data, 'items'):
            result = []
            for k, vs in list(data.items()):
                for v in isinstance(vs, list) and vs or [vs]:
                    # Hotfix to resolve https://github.com/kennethreitz/requests/issues/378
                    if v == None:
                        result.append((k.encode('utf-8') if isinstance(k, str) else k,
                                   ""))
                    else:
                        result.append((k.encode('utf-8') if isinstance(k, str) else k,
                                   v.encode('utf-8') if isinstance(v, str) else v))

            return result, urlencode(result, doseq=True)
        else:
            return data, data

```


@foxx If you want to send empty string as a value of some param you should set its value to empty string. Why should requests send `key=''` when you pass `key=None` and not skip this param altogether? 
btw, using `==` when comparing with `None` is bad style.


> Why should requests send key='' when you pass key=None and not skip this param altogether? 

Guess my reply is, why SHOULDN'T it? It's a 50/50 split, and really comes down to the decision of the core maintainer I guess.

> using == when comparing with None is bad style.

You're right, I probably should have used "is None" - I'll patch our side - thanks.


It's not 50/50 split if you take into consideration some other aspects. In case of params requests already deletes keys with value set to `None` so for sake of being uniform here it should delete them too.


Seven months later, resolved by #805!


:trollface: