Skip to content

Yapf linting #1872

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

Merged
merged 3 commits into from
Apr 11, 2018
Merged

Yapf linting #1872

merged 3 commits into from
Apr 11, 2018

Conversation

pcmoritz
Copy link
Contributor

This is linting the Python codebase with yapf and automatically checking on travis if files have linting errors. Currently excludes rllib and dataframes.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/4788/
Test FAILed.

Copy link
Contributor

@concretevitamin concretevitamin left a comment

Choose a reason for hiding this comment

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

LGTM feel free to merge if tests pass

@robertnishihara
Copy link
Collaborator

@richardliaw note that this currently does linting for Tune (but not RLlib or Pandas on Ray). Is now a reasonable time for tune?

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/4801/
Test FAILed.

@richardliaw
Copy link
Contributor

Tune is fine right now; thanks for checking!

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/4803/
Test PASSed.

@robertnishihara robertnishihara merged commit 74162d1 into ray-project:master Apr 11, 2018
@robertnishihara robertnishihara deleted the yapf-linting branch April 11, 2018 17:11
This was referenced Apr 12, 2018
@alok
Copy link
Contributor

alok commented Apr 12, 2018

I'd tweak the style file to

  1. split before the first argument on long function calls since it makes it easy to quickly scan the argument list (split_before_first_argument = True)
  2. not join lines to make single line if statements (breaks visual flow since you usually expect the indent) (join_multiple_lines = False)
  3. always split if comma terminated since the clean diffs created by trailing commas only works if there's one item per line (split_arguments_when_comma_terminated = True)

But those are just minor nitpicks. Glad to see this.

royf added a commit to royf/ray that referenced this pull request Apr 22, 2018
* master: (56 commits)
  [xray] Turn on flushing to the GCS for the lineage cache (ray-project#1907)
  Single Big Object Parallel Transfer. (ray-project#1827)
  Remove num_threads as a parameter. (ray-project#1891)
  Adds Valgrind tests for multi-threaded object manager. (ray-project#1890)
  Pin cython version in docker base dependencies file. (ray-project#1898)
  Update arrow to efficiently serialize more types of numpy arrays. (ray-project#1889)
  updates (ray-project#1896)
  [DataFrame] Inherit documentation from Pandas (ray-project#1727)
  Update arrow and parquet-cpp. (ray-project#1875)
  raylet command line resource configuration plumbing (ray-project#1882)
  use raylet for remote ray nodes (ray-project#1880)
  [rllib] Propagate dim option to deepmind wrappers (ray-project#1876)
  [RLLib] DDPG (ray-project#1685)
  Lint Python files with Yapf (ray-project#1872)
  [DataFrame] Fixed repr, info, and memory_usage (ray-project#1874)
  Fix getattr compat (ray-project#1871)
  check if arrow build dir exists (ray-project#1863)
  [DataFrame] Encapsulate index and lengths into separate class (ray-project#1849)
  [DataFrame] Implemented __getattr__ (ray-project#1753)
  Add better analytics to docs (ray-project#1854)
  ...

# Conflicts:
#	python/ray/rllib/__init__.py
#	python/setup.py
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.

6 participants