Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pep8ing again because rebasing kills a pull request #13

Merged
merged 4 commits into from
Apr 18, 2016

Conversation

cad106uk
Copy link

following on from issue #12

Sorry I didn't realise that rebasing would kill a PR. You live and learn.

So after fixing one nasty circular import and a couple of minor bugs. And remembering to git clean -xdf before running the tests. Here is the second attempt.

In func.py and op.py there was a circular import that has been removed
In immutable/heap.py there was a logic bug introduced by me (fixed)
In iters.py the .uniform module is used to make P2 and P3 work the same
way. can't make this PEP8 compilent. Have to `import *`
@jacobbridges
Copy link

Thank you for going through all this trouble to fix some of the PEP8 things.

This PR seems to fail on all Python 3 versions, but works fine on Python 2. https://travis-ci.org/fnpy/fn.py/jobs/119619343#L225

@microamp
Copy link
Member

Python 3 versions are failing because reduce is no longer a built-in in Python 3.

Regardless, I think as a team, we need to agree on what linter and formatter to use. I personally use pylint and yapf. We can easily set them up as we like it. e.g.

I use the same settings globally, but different settings can be used for different projects. IIRC, you just need to place these dotfiles under the project root. That would override the global settings.

@cad106uk
Copy link
Author

Yep, pushed a fix for the reduce function

@cad106uk
Copy link
Author

I am currently using flake8, but I am adaptable. flake8

@jacobbridges
Copy link

@cad106uk I apologize for the delay on responding to this. I've pulled down your branch on my local machine and ran a code inspection against it with flake8 -- it still shows 318 errors? Were you only fixing certain PEP-8 errors?

I can always merge this into master then create a new PR to fix the rest of the errors.

@cad106uk
Copy link
Author

I have gone back through this. There where some line length errors I had to fix.

There are unused imports flake8 errors in the files ./fn/init.py , ./fn/uniform.py and ./fn/immutable/init.py . These imports are used through out the codebase but are not used in these respective files. I cannot see an easy way to refactor the code to deal with these flake8 errors, I think it might be better to make these errors an exception. What do you think?

I haven't made any changes to the tests.py file where most of the errors are. This file is going to be split into a module anyway.

@cad106uk
Copy link
Author

Do we have any thoughts on this?

@jacobbridges
Copy link

Hi @cad106uk, I will address these issues in a future refactoring pull request. Thank you for contributing to the refactor of this project, and I apologize for not responding sooner. Some weeks my schedule is very hectic.

@jacobbridges jacobbridges merged commit 5d9238a into fnpy:master Apr 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants