pylint friendly

Kindly request that requests be more pylint compliant.  When evaluating which package to use, this comes up with a score of -154.
## Global evaluation

Your code has been rated at -154.92/10 (previous run: -154.91/10, -0.00)

Errors:

Davids-MacBook-Pro-2:~ dhwu$ pylint -E /Users/dhwu/Desktop/Evals/HTTP\ Libs/requests-2.5.1/requests 
No config file found, using default configuration
************\* Module requests.compat
E: 88, 4: No name 'quote' in module 'urllib' (no-name-in-module)
E: 88, 4: No name 'unquote' in module 'urllib' (no-name-in-module)
E: 88, 4: No name 'quote_plus' in module 'urllib' (no-name-in-module)
E: 88, 4: No name 'unquote_plus' in module 'urllib' (no-name-in-module)
E: 88, 4: No name 'urlencode' in module 'urllib' (no-name-in-module)
E: 88, 4: No name 'getproxies' in module 'urllib' (no-name-in-module)
E: 88, 4: No name 'proxy_bypass' in module 'urllib' (no-name-in-module)
E: 98,10: Undefined variable 'unicode' (undefined-variable)
E: 99,17: Using variable 'basestring' before assignment (used-before-assignment)
E:100,26: Undefined variable 'long' (undefined-variable)
************\* Module requests.models
E: 39,10: Instance of 'LookupDict' has no 'moved' member (no-member)
E: 40,10: Instance of 'LookupDict' has no 'found' member (no-member)
E: 41,10: Instance of 'LookupDict' has no 'other' member (no-member)
E: 42,10: Instance of 'LookupDict' has no 'temporary_redirect' member (no-member)
E: 43,10: Instance of 'LookupDict' has no 'permanent_redirect' member (no-member)
E:343,18: Undefined variable 'unicode' (undefined-variable)
E:634,74: Instance of 'LookupDict' has no 'moved_permanently' member (no-member)
E:634,99: Instance of 'LookupDict' has no 'permanent_redirect' member (no-member)
E:792,51: Instance of 'bool' has no 'decode' member (no-member)
************\* Module requests.sessions
E:145,42: Instance of 'LookupDict' has no 'see_other' member (no-member)
E:151,41: Instance of 'LookupDict' has no 'found' member (no-member)
E:156,41: Instance of 'LookupDict' has no 'moved' member (no-member)
E:162,46: Instance of 'LookupDict' has no 'temporary_redirect' member (no-member)
E:162,72: Instance of 'LookupDict' has no 'permanent_redirect' member (no-member)
************\* Module requests.utils
E:537,52: Module 'sys' has no 'pypy_version_info' member (no-member)
E:538,52: Module 'sys' has no 'pypy_version_info' member (no-member)
E:539,52: Module 'sys' has no 'pypy_version_info' member (no-member)
E:540,15: Module 'sys' has no 'pypy_version_info' member (no-member)
E:541,76: Module 'sys' has no 'pypy_version_info' member (no-member)
************\* Module requests.packages.chardet
E: 23,52: Undefined variable 'unicode' (undefined-variable)
************\* Module requests.packages.chardet.compat
E: 25,21: Undefined variable 'unicode' (undefined-variable)
************\* Module requests.packages.urllib3.connection
E:159, 0: class already defined line 20 (function-redefined)
************\* Module requests.packages.urllib3.connectionpool
E: 46,19: Instance of '_MovedItems' has no 'xrange' member (no-member)
************\* Module requests.packages.urllib3.request
E:  4, 4: No name 'urlencode' in module 'urllib' (no-name-in-module)
E: 49, 8: NotImplemented raised - should raise NotImplementedError (notimplemented-raised)
************\* Module requests.packages.urllib3.response
E:182,36: Instance of 'str' has no 'read' member (no-member)
E:186,36: Instance of 'str' has no 'read' member (no-member)
E:195,33: Instance of 'str' has no 'close' member (no-member)
E:296,21: Instance of 'str' has no 'close' member (no-member)
E:303,28: Instance of 'str' has no 'closed' member (no-member)
E:305,28: Instance of 'str' has no 'isclosed' member (no-member)
E:313,28: Instance of 'str' has no 'fileno' member (no-member)
E:320,28: Instance of 'str' has no 'flush' member (no-member)
************\* Module requests.packages.urllib3.contrib.ntlmpool
E: 44,13: Instance of 'NTLMConnectionPool' has no 'num_connections' member (no-member)
E: 46,24: Instance of 'NTLMConnectionPool' has no 'num_connections' member (no-member)
E: 46,46: Instance of 'NTLMConnectionPool' has no 'host' member (no-member)
E: 53,41: Instance of 'NTLMConnectionPool' has no 'host' member (no-member)
E: 53,57: Instance of 'NTLMConnectionPool' has no 'port' member (no-member)
************\* Module requests.packages.urllib3.contrib.pyopenssl
E: 58, 0: No name '_fileobject' in module 'socket' (no-name-in-module)
E:158,33: Instance of 'OctetString' has no 'getComponentByPosition' member (no-member)
E:158,33: Instance of 'Any' has no 'getComponentByPosition' member (no-member)
E:158,33: Instance of 'str' has no 'getComponentByPosition' member (no-member)
************\* Module requests.packages.urllib3.packages.ordered_dict
E: 82,35: Instance of 'dict' has no 'itervalues' member (no-member)
E:142, 4: Method has no argument (no-method-argument)
E:165,29: Instance of 'tuple' has no 'keys' member (no-member)
************\* Module requests.packages.urllib3.packages.six
E: 42,19: Undefined variable 'basestring' (undefined-variable)
E: 43,26: Undefined variable 'long' (undefined-variable)
E: 44,31: Module 'types' has no 'ClassType' member (no-member)
E: 45,16: Undefined variable 'unicode' (undefined-variable)
E: 84,22: Instance of '_LazyDescr' has no '_resolve' member (no-member)
E:294,15: Undefined variable 'unicode' (undefined-variable)
E:341,36: Undefined variable 'basestring' (undefined-variable)
E:347,31: Undefined variable 'unicode' (undefined-variable)
E:353,31: Undefined variable 'unicode' (undefined-variable)
E:361,35: Undefined variable 'unicode' (undefined-variable)
E:365,22: Undefined variable 'unicode' (undefined-variable)
E:366,20: Undefined variable 'unicode' (undefined-variable)
************\* Module requests.packages.urllib3.util.connection
E: 87, 8: Raising NoneType while only classes or instances are allowed (raising-bad-type)
************\* Module requests.packages.urllib3.util.ssl_
E: 22, 4: No name 'OP_NO_SSLv2' in module 'ssl' (no-name-in-module)
E: 22, 4: No name 'OP_NO_SSLv3' in module 'ssl' (no-name-in-module)
E: 22, 4: No name 'OP_NO_COMPRESSION' in module 'ssl' (no-name-in-module)
E: 41, 4: class already defined line 7 (function-redefined)
Davids-MacBook-Pro-2:~ dhwu$ 


Opening this kind of issue is exactly the kind of thing that is likely to get you some very negative comments indeed. =) Offering criticism without offering to help improve it is extremely bad manners. It's worse when the criticism is of the form "Your project doesn't live up to my personal standard <x>" as this is. It's worse still when your criticism is based on an automated tool that is _wrong_.

Allow me to pick some examples:

> E: 88, 4: No name 'quote' in module 'urllib' (no-name-in-module)

That's only true in Python 3, and the whole purpose of this module is to cover over differences between Python 2 and 3. All these errors are wrong.

> E: 39,10: Instance of 'LookupDict' has no 'moved' member (no-member)

Static analysis of Python is _stupid_: it's a dynamic language! In practice, the instance gains this member at import time: any tool smart enough to import the module would have known this. These errors are all wrong.

> E:537,52: Module 'sys' has no 'pypy_version_info' member (no-member)

It does on PyPy. Given that this is guarded by an 'if pypy' check, this error and all the ones like it are wrong.

> E: 23,52: Undefined variable 'unicode' (undefined-variable)

There is on Python 2.

Any of the errors on urllib3 are in the wrong module, which is even more obviously true of `six`.

I'm being as polite as I can when I say that your tool is doing a _terrible_ job. I strongly advise you never to open a similar issue on a different repository, or you will likely encounter a much stronger negative reaction than this one.


It looks like most of these are based around py2/3 compatibility which we have no intention of breaking. 


@Lukasa : I am following a well known practice that is detailed in Idiomatic Python author:
http://www.jeffknupp.com/blog/2013/11/15/supercharge-your-python-developers/
Also detailed in Google coding standards.
FWIW:  pylint has caught many errors in my code and others in the past.
While we may differ in our views, not placing value judgements.  Just a data point.
Like I said in my prior post, my intent is to weight differing approaches.  Request at the top of that list.

Was hoping that this kind ask would be more open to possible changes... looks like closed for discussion already.

If you guys are open, I'd be open to making changes once I had more time.
But then that would be un-welcomed based on your statement too.
my 2 cents


To be clear: I do not believe anything that pylint has printed for modules outside `requests.packages` (which are third party code) is valid. I think everything it has said is wrong. If you can point to something that is not wrong, I will happily consider accepting a change based on it.

I agree that pylint can catch errors. However, doing a drive-by dump of every line reported by pylint is not a helpful approach. Two notes for the future:
1. Authors often have different views on Python style, particularly PEP8. Requests does not follow all of PEP8, and this is quite deliberate. You should _always_ consult a project's author before raising a bug report about their code failing a linter.
2. You should validate the concerns of the linter before raising them. Open source maintainers are often very busy people, and asking them to validate each line of the linter's output for false positives is a rude way to behave. If you'd looked at these 'errors' first you'd have seen that they aren't valid.
3. It's likely that you'll say we should add a `.pylintrc` file to address these concerns. That's not a good idea. We should not have to add a dotfile to the repository root for every person who writes a linter.


Points taken

My apology for this faux pas

Sent from my iPhone

> On Jan 20, 2015, at 10:19 AM, Cory Benfield notifications@github.com wrote:
> 
> To be clear: I do not believe anything that pylint has printed for modules outside requests.packages (which are third party code) is valid. I think everything it has said is wrong. If you can point to something that is not wrong, I will happily consider accepting a change based on it.
> 
> I agree that pylint can catch errors. However, doing a drive-by dump of every line reported by pylint is not a helpful approach. Two notes for the future:
> 
> Authors often have different views on Python style, particularly PEP8. Requests does not follow all of PEP8, and this is quite deliberate. You should always consult a project's author before raising a bug report about their code failing a linter.
> You should validate the concerns of the linter before raising them. Open source maintainers are often very busy people, and asking them to validate each line of the linter's output for false positives is a rude way to behave. If you'd looked at these 'errors' first you'd have seen that they aren't valid.
> It's likely that you'll say we should add a .pylintrc file to address these concerns. That's not a good idea. We should not have to add a dotfile to the repository root for every person who writes a linter.
> —
> Reply to this email directly or view it on GitHub.


@Lukasa 
Are you open to annotating source with comments disabling pylint warnings? Given that one [can](http://docs.pylint.org/faq.html#do-i-have-to-remember-all-these-numbers) use symbolic names of warnings/errors such annotation would serve as documentation at the same time which should be useful.


@piotr-dobrogost Nope. =)

My concern is the potentially unbounded number of linters that we may be asked to support annotations or dotfiles for. Just because someone created a tool that they love doesn't mean we should add metadata to our codebase to support it. If the requests project uses a tool for its development, we'll add that metadata. Otherwise, we won't.


@piotr-dobrogost Are you open to determining how many of those are false positives and constitute absolutely useless feedback from a linter? The majority of the errors are just plain **wrong** and any changes belong in pylint.


https://github.com/PyCQA/pylint/issues/1411 is the pylint issue which unfortunately indicates that pylint will not support `requests.codes`.

However, a workaround has been suggested for users: `generated-members`, documented in https://pylint.readthedocs.io/en/latest/technical_reference/features.html#id34.

To be clear, Requests should not add `generated-members` support itself. If folks want to add it to their own CI that’s fine, but pylint’s failure to support this does not place a burden on us to support them.

@Lukasa - yes, I wrote that comment to try to be helpful for people coming to this issue with the same issue that I had.

The hack that I did as a good-enough-for-me workaround was to run:

```python
import string

import requests

pattern = '(requests\.)?codes\.({allowed}),'
accepted_characters = set(string.ascii_letters + '_')
keys = [key for key in requests.codes.__dict__.keys() if set(key).issubset(accepted_characters)]
print(pattern.format(allowed='|'.join(keys)))
```

and then add the outputted string to my `pylint-rc`'s `generated-members` section.