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

STY: Fix new pep8 errors from pep8's update #819

Merged
merged 2 commits into from
Feb 12, 2015

Conversation

ebolyen
Copy link
Contributor

@ebolyen ebolyen commented Feb 12, 2015

fixes #818

@jairideout jairideout added this to the 0.3.0: The Contingency Plan milestone Feb 12, 2015
@@ -627,7 +627,9 @@ def update_ids(self, ids=None, fn=None, prefix=""):
"or fn is provided.")

if ids is not None:
fn = lambda _: ids
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is frowned upon now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would seem so.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, pep8 now disallows storing lambdas in variables.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's unfortunate.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's ridiculous ...

On (Feb-12-15| 9:32), Evan Bolyen wrote:

@@ -627,7 +627,9 @@ def update_ids(self, ids=None, fn=None, prefix=""):
"or fn is provided.")

     if ids is not None:
  •        fn = lambda _: ids
    

It would seem so.


Reply to this email directly or view it on GitHub:
https://github.com/biocore/scikit-bio/pull/819/files#r24599822

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😢

However, from the PEP8 example, maybe we can keep those super simple functions in one line

Always use a def statement instead of an assignment statement that binds a lambda expression directly to an identifier.
Yes:
def f(x): return 2*x.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a really good suggestion!

On (Feb-12-15| 9:43), Jorge Ca�ardo Alastuey wrote:

@@ -627,7 +627,9 @@ def update_ids(self, ids=None, fn=None, prefix=""):
"or fn is provided.")

     if ids is not None:
  •        fn = lambda _: ids
    

😢

However, from the PEP8 example, maybe we can keep those super simple functions in one line

Always use a def statement instead of an assignment statement that binds a lambda expression directly to an identifier.
Yes:
def f(x): return 2*x.


Reply to this email directly or view it on GitHub:
https://github.com/biocore/scikit-bio/pull/819/files#r24600722

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not know that was possible. Will update once I get travis back into a working state.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, from the PEP8 example, maybe we can keep those super simple functions in one line

I'm going to merge this so we can get things passing again. Using one line functions in the future sounds like a good idea but I don't think it's too critical at this point. If someone's available to work on this in a separate PR that'd be great!

@@ -14,9 +14,6 @@
import warnings

import numpy as np

from matplotlib import use
use('Agg', warn=False)
import matplotlib.pyplot as plt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the use command must run before importing pyplot...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working on a way around this issue now...

@ebolyen
Copy link
Contributor Author

ebolyen commented Feb 12, 2015

I've built the docs locally and can confirm that the images are still rendered correctly.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) to 98.87% when pulling 6e3f6f9 on ebolyen:issue-818 into 1f3bb5a on biocore:master.

@jairideout jairideout self-assigned this Feb 12, 2015
jairideout added a commit that referenced this pull request Feb 12, 2015
STY: Fix new pep8 errors from pep8's update
@jairideout jairideout merged commit adfdb5e into scikit-bio:master Feb 12, 2015
@Jorge-C
Copy link
Contributor

Jorge-C commented Feb 12, 2015

Thank you guys! 💯

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.

pep8 has been updated
6 participants