CLN run isort on source code files

[MarcoGorelli]

isort is run on notebooks, but not on Python files. Should be pretty straightforward to run it on the source code and add it as a pre-commit hook

[chandan5362]

@MarcoGorelli Please provide a little more information about it. Any error snippet would be quite helpful. I would like to work on it. Please assign it to me.

[MarcoGorelli]

Sure - if you look at https://github.com/pandas-dev/pandas/blob/master/.pre-commit-config.yaml you'll see that there's a hook called 'isort'. PyMC3 doesn't have it yet - there's a similar hook called nbqa-isort, but that only runs on notebooks

Steps are:

1. add the same hook to .pre-commit-config.yaml in pymc3
2. run pre-commit run isort --all-files
3. stage and commit the changed files, push to a branch
4. review the changes - if they look sensible, open a pull request

If any of this is unclear, check the contributing guide https://github.com/pymc-devs/pymc3/blob/master/CONTRIBUTING.md , and if it's still unclear, please don't hesitate to ask for help! 😄

[chandan5362]

@MarcoGorelli I am trying to rebase but I am getting merge conflict.
Here is the snippet

First, rewinding head to replay your work on top of it...
Applying: isort added for .py files
Using index info to reconstruct a base tree...
M	pymc3/__init__.py
M	pymc3/backends/__init__.py
M	pymc3/backends/base.py
A	pymc3/backends/hdf5.py
A	pymc3/backends/sqlite.py
A	pymc3/backends/text.py
M	pymc3/sampling.py
A	pymc3/tests/test_hdf5_backend.py
M	pymc3/tests/test_sampling.py
A	pymc3/tests/test_sqlite_backend.py
A	pymc3/tests/test_text_backend.py
M	pymc3/tests/test_variational_inference.py
M	pymc3/variational/opvi.py
Falling back to patching base and 3-way merge...
Auto-merging pymc3/variational/opvi.py
CONFLICT (content): Merge conflict in pymc3/variational/opvi.py
Auto-merging pymc3/tests/test_variational_inference.py
CONFLICT (modify/delete): pymc3/tests/test_text_backend.py deleted in HEAD and modified in isort added for .py files. Version isort added for .py files of pymc3/tests/test_text_backend.py left in tree.
CONFLICT (modify/delete): pymc3/tests/test_sqlite_backend.py deleted in HEAD and modified in isort added for .py files. Version isort added for .py files of pymc3/tests/test_sqlite_backend.py left in tree.
Auto-merging pymc3/tests/test_sampling.py
CONFLICT (modify/delete): pymc3/tests/test_hdf5_backend.py deleted in HEAD and modified in isort added for .py files. Version isort added for .py files of pymc3/tests/test_hdf5_backend.py left in tree.
Auto-merging pymc3/sampling.py
CONFLICT (modify/delete): pymc3/backends/text.py deleted in HEAD and modified in isort added for .py files. Version isort added for .py files of pymc3/backends/text.py left in tree.
CONFLICT (modify/delete): pymc3/backends/sqlite.py deleted in HEAD and modified in isort added for .py files. Version isort added for .py files of pymc3/backends/sqlite.py left in tree.
CONFLICT (modify/delete): pymc3/backends/hdf5.py deleted in HEAD and modified in isort added for .py files. Version isort added for .py files of pymc3/backends/hdf5.py left in tree.
Auto-merging pymc3/backends/base.py
Auto-merging pymc3/backends/__init__.py
CONFLICT (content): Merge conflict in pymc3/backends/__init__.py
Auto-merging pymc3/__init__.py
CONFLICT (content): Merge conflict in pymc3/__init__.py
error: Failed to merge in the changes.
Patch failed at 0001 isort added for .py files
hint: Use 'git am --show-current-patch' to see the failed patch
Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".

Should I abort the rebasing or go for manually resolving the conflicts??

[MarcoGorelli]

It's not clear to me why you would get conflicts, there shouldn't be any need to resolve them for this issue - I've put some suggested steps in #4239 anyway

[chandan5362]

Now i know my mistake. I guess, I forgot to git pull before making changes. Thanks a lot @MarcoGorelli

[chandan5362]

Also , while running pytest, i get these errors -

ImportError while loading conftest '/home/travis/build/pymc-devs/pymc3/pymc3/tests/conftest.py'.
pymc3/__init__.py:41: in <module>
    from . import gp, ode, sampling
pymc3/gp/__init__.py:15: in <module>
    from . import cov, mean, util
pymc3/gp/util.py:22: in <module>
    cholesky = tt.slinalg.cholesky
E   AttributeError: module 'theano.tensor' has no attribute 'slinalg'

I ran through the web and it was mentioned that we have to explicitly import the theano.tensor.slinalg.
is it??

[MarcoGorelli]

@chandan5362 could you show the exact command you ran please?

[chandan5362]

Here I a attaching the screenshot.

and also please go through these my PR check logs. Same error is causing the checks to fail.

[MarcoGorelli]

you wouldn't typically run pytest on contest.py itself - conftest.py just contains the fixures. You'd run, e.g.

$ pytest pymc3/tests/test_memo.py 

Anyway, you shouldn't need to run pytest for this particular issue (sorting imports shouldn't affect test results, though the tests are run anyway during continuous integration just to be sure)