Preparatory changes for an API version 2.0

[pcostell]

This does a few things that should help on a 2.0 API. In particular:

• lint rules -- I'm happy to change what is here, they are mostly the defaults plus some exceptions to rules that I thought were not necessary.
• more test coverage -- I added several tests and also converted the existing doctests to unittests. Some of them probably could have been left as is, but I thought having all the code in doc strings made it harder to follow.
• private functions -- I moved all the functions that were marked as private helpers to begin with an underscore. Moving these out of the API should allow more freedom to change implementation details without bumping the major version.
• logging -- I converted any un-requested printing to use a logger. I say un-requested because show_growth and show_most_common_types still use print statements.

[coveralls]

Coverage increased (+2.36%) to 99.72% when pulling 9213e4a35e46bf593397ed785a12da4c37910b0d on pcostell:pylint into 807a940fa53c316fc3b6ae7bb255fcfcdd710e45 on mgedmin:master.

[coveralls]

Coverage increased (+2.36%) to 99.72% when pulling 9213e4a35e46bf593397ed785a12da4c37910b0d on pcostell:pylint into 807a940fa53c316fc3b6ae7bb255fcfcdd710e45 on mgedmin:master.

[mgedmin]

Thank you! There are a lot of changes, so I'm afraid I might be slower reviewing them.

I'm confused by the first commit of this PR (d945c9c4c33e98f2038da660d4f989d4ef0c1fab). Haven't I merged an equivalent change to master already? Would you mind rebasing on top of master, so that I can review this commit-by-commit?

Then there's 98d683b5c76bc4a7dcd2846d52409e9279f84cb3, which claims to be about adding pylint rules, but it changes a lot of unrelated things as well. Is it reformatting the code to make it lint-free? It's a huge change that's hard to review; can you split it? Also, I'm not sure I'll like all the changes, e.g. the 2-space indentation of doctest assertion feels wrong to me.

(I personally use flake8, mostly because pylint bombards me with a deluge of assertions and I'd have to spend time configuring it to make it useful.)

[pcostell]

Ya. Why don't we just drop this pull request and I'll send you a couple smaller ones. I made a bunch of changes playing around with things and it somehow made it into this branch.

Do you prefer flake8? I'm happy to drop the pylint stuff if you'd rather have flake8? I'm also happy to put in more pylint rules to get rid of the dumb stuff.

-Patrick

On Sat, Feb 21, 2015, 5:38 AM Marius Gedminas notifications@github.com wrote:

Thank you! There are a lot of changes, so I'm afraid I might be slower reviewing them.

I'm confused by the first commit of this PR (d945c9c https://github.com/mgedmin/objgraph/commit/d945c9c4c33e98f2038da660d4f989d4ef0c1fab). Haven't I merged an equivalent change to master already? Would you mind rebasing on top of master, so that I can review this commit-by-commit?

Then there's 98d683b https://github.com/mgedmin/objgraph/commit/98d683b5c76bc4a7dcd2846d52409e9279f84cb3, which claims to be about adding pylint rules, but it changes a lot of unrelated things as well. Is it reformatting the code to make it lint-free? It's a huge change that's hard to review; can you split it? Also, I'm not sure I'll like all the changes, e.g. the 2-space indentation of doctest assertion feels wrong to me.

(I personally use flake8, mostly because pylint bombards me with a deluge of assertions and I'd have to spend time configuring it to make it useful.)

— Reply to this email directly or view it on GitHub https://github.com/mgedmin/objgraph/pull/13#issuecomment-75366029.

[mgedmin]

My text editor (vim) is configured to show me flake8 warnings for Python files. I use it for all of the projects. I'm not going to switch to pylint, if that means I have to invest time in crafting pylint config files for all the projects where I already have [flake8] sections in setup.cfg suppressing over-eager warnings.

flake8 combines the warnings from several tools: pyflakes, which I care about because it finds real errors, and pep8, which I sometimes find annoying, because it violates the PEP 8 rule about PEP 8 not being cast in stone.

flake8 currently reports one complaint about setup.py (multiple imports on one line) and three complaints about objgraph.py (missing whitespace around operator, that actually makes the expression more readable, IMHO):

    h = h1 * (1-f) + h2 * f
    s = s1 * (1-f) + s2 * f
    v = v1 * (1-f) + v2 * f

I haven't decided what I want to do about it yet (give in and add the space? add ugly # noqa comments? suppress the warning in setup.cfg?), and so it remains. Likewise the setup.py one: I probably should split the imports, but it doesn't bug me as much as, say, trailing whitespace or the wrong number of blank lines between functions would bug me.

Given all that, I like the idea of adding a .pylintrc and make lint, but I'm disinclined to blindly reformat existing code to make pylint happy. I'm sure there are stylistic improvements that can be made (e.g. some lines are longer than 80 columns for no good reason -- and why doesn't flake8 complain? probably my global flake8.cfg), but I'd like to consider them one-by-one.