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

Satisfy pylint #252

Merged
merged 8 commits into from Jun 22, 2018

Conversation

Projects
None yet
4 participants
@benjamin-work
Copy link
Collaborator

commented Jun 14, 2018

  • non-functional changes for pylint
  • allow attribute names of type "X_*"
  • allow line lengths up to 88

@benjamin-work benjamin-work self-assigned this Jun 14, 2018

@benjamin-work benjamin-work requested a review from ottonemo Jun 14, 2018

@taketwo

This comment has been minimized.

Copy link
Contributor

commented Jun 14, 2018

I finally got around to start implementing the things discussed in #246. What line length should I format for? In general, how do you format code? Yapf?

@BenjaminBossan

This comment has been minimized.

Copy link
Collaborator

commented Jun 14, 2018

We don't currently have strict rules in place but if you go with black you probably won't go wrong. There might be some minor things I disagree with, but overall it looks very reasonable.

@taketwo

This comment has been minimized.

Copy link
Contributor

commented Jun 14, 2018

The problem is that the skorch codebase is not formatted with black (yet?). Thus, if I format with black, my patch will include style changes in unrelated places. We probably want to avoid this, so I will not commit those unrelated changes?

@ottonemo

This comment has been minimized.

Copy link
Collaborator

commented Jun 14, 2018

Yes, better leave those out to avoid cluttering the diff.

@ottonemo ottonemo merged commit 00f53ec into master Jun 22, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@ottonemo ottonemo deleted the feature/satisfy-pylint branch Jul 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.