Use flake8-comprehensions by alok · Pull Request #1976 · ray-project/ray

alok · 2018-05-01T18:57:50Z

What do these changes do?

flake8-comprehensions checks for idiomatic (and slightly more efficient) usage
of comprehensions and builtins.

Add flake8 and flake8 comprehensions to travis
Lint all files and apply changes that were not covered in Clean up syntax for supported Python versions. #1963.

Related issue number

No.

[flake8 plugin](https://github.com/adamchainz/flake8-comprehensions) that checks for useless constructions.

A lot of the builtins can take in generators instead of lists. This commit applies `flake8-comprehensions` to find them.

AmplabJenkins · 2018-05-01T20:02:53Z

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

The rest can be fixed in another PR

This should probably be merged after ray-project#1963.

AmplabJenkins · 2018-05-01T22:52:35Z

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

* master: (25 commits) [DataFrame] Add direct pandas imports for MVP (ray-project#1960) Make ActorHandles pickleable, also make proper ActorHandle and ActorC… (ray-project#2007) Expand local_dir in Trial init (ray-project#2013) Fixing ascii error for Python2 (ray-project#2009) [DataFrame] Implements df.update (ray-project#1997) [DataFrame] Implements df.as_matrix (ray-project#2001) [DataFrame] Implement quantile (ray-project#1992) [DataFrame] Impement sort_values and sort_index (ray-project#1977) [DataFrame] Implement rank (ray-project#1991) [DataFrame] Implemented prod, product, added test suite (ray-project#1994) [DataFrame] Implemented __setitem__, select_dtypes, and astype (ray-project#1941) [DataFrame] Implement diff (ray-project#1996) [DataFrame] Implemented nunique, skew (ray-project#1995) [DataFrame] Implements filter and dropna (ray-project#1959) [DataFrame] Implements df.pipe (ray-project#1999) [DataFrame] Apply() for Lists and Dicts (ray-project#1973) Clean up syntax for supported Python versions. (ray-project#1963) [DataFrame] Implements mode, to_datetime, and get_dummies (ray-project#1956) [DataFrame] Fix dtypes (ray-project#1930) keep_dims -> keepdims (ray-project#1980) ...

AmplabJenkins · 2018-05-09T22:01:55Z

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

pcmoritz · 2018-05-11T00:51:45Z

I like the changes in this PR, but there is a number of linting failures in the Travis test that should be fixed before we can merge it.

* master: [xray] Fix UniqueID hashing for object and task IDs. (ray-project#2017) [DataFrame] Fixing bugs in groupby (ray-project#2031) [DataFrame] Fixes dropna subset bug (ray-project#2018) [DataFrame] Implement where (ray-project#1989)

dict(...) -> {...}

AmplabJenkins · 2018-05-11T10:56:48Z

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

richardliaw · 2018-05-14T08:30:40Z

lint still failing?

AmplabJenkins · 2018-05-14T20:50:49Z

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

AmplabJenkins · 2018-05-15T00:34:07Z

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

* master: Create RemoteFunction class, remove FunctionProperties, simplify worker Python code. (ray-project#2052) Don't crash on duplicate actor notifications (ray-project#2043) Fixed attribute name in code example (ray-project#2054) [xray] Add Travis build for testing xray on Linux. (ray-project#2047) Added missing comma to code example (ray-project#2050) Use more CPUs for testMultipleWaitsAndGets. (ray-project#2051) use jobid_nil (ray-project#2044) Fix typo in tune. (ray-project#2046) Fix error in api.rst. (ray-project#2048) Improve shared_ptr usage (ray-project#2030)

This is already installed in another file.

AmplabJenkins · 2018-05-15T01:58:10Z

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

AmplabJenkins · 2018-05-15T02:09:11Z

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

richardliaw · 2018-05-15T02:59:16Z

python/ray/rllib/models/catalog.py


    @staticmethod
-    def get_preprocessor_as_wrapper(registry, env, options=dict()):
+    def get_preprocessor_as_wrapper(registry, env, options={}):


Options should probably be None, and resolve within the function.

There was a prior issue about putting dict objects in the default arguments...

I agree, though I think that would be best done in another PR, since it would require writing extra logic to handle the None.

richardliaw · 2018-05-15T03:00:36Z

python/ray/rllib/dqn/apex.py

+                    'num_replay_buffer_shards': 4,
+                    'debug': False
+                }),
+        'n_step':


this is odd formatting..

Fixed in last commit.

richardliaw · 2018-05-15T03:10:10Z

python/ray/rllib/ddpg/apex.py

+    return d
+
+
+APEX_DDPG_DEFAULT_CONFIG = merge_dicts(


I think we don't need merge_dicts.

You can probably just do

APEX_DDPG_DEFAULT_CONFIG = DDPG_CONFIG.copy() APEX_DDPG_DEFAULT_CONFIG.update({...})

Also, the formatting for the dict is not really nice - let's make sure keys and values are on the same line.

I thought about the .copy and .update but it made updating the inner dict (optimizer config) awkward. In python 3.5, you can just do new_dict = {**old1,**old2}, but this doesn't work on 2.7.

Oh, I see. In that case let's keep as is and then change merge_dicts to a utility function (in rllib/utils).

robertnishihara · 2018-05-15T22:19:45Z

We should also turn on yapf for rllib/tune/pandas-on-ray (in a separate PR of course).

alok · 2018-05-15T22:29:54Z

@robertnishihara What about enabling it for the rest of rllib?

richardliaw · 2018-05-15T22:36:32Z

I think it's just not turned on right now.

…

On Tue, May 15, 2018 at 3:30 PM Alok Singh ***@***.***> wrote: @robertnishihara <https://github.com/robertnishihara> What about enabling it for the rest of rllib? — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#1976 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEUc5UM8zYkEBDkG46b7pvTuKGjOwCVXks5ty1bqgaJpZM4TuWBM> .

alok · 2018-05-16T17:36:46Z

@robertnishihara I can enable yapf for those directories in #1971 since it's not merged anyway.

By the way, is pandas-on-ray in the dataframe/ dir?

AmplabJenkins · 2018-05-16T18:24:35Z

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

* master: (22 commits) [xray] Fix bug in updating actor execution dependencies (ray-project#2064) [DataFrame] Refactor __delitem__ (ray-project#2080) [xray] Better error messaging when pulling from self. (ray-project#2068) Use source code in hash where possible (fix ray-project#2089) (ray-project#2090) Functions for flushing done tasks and evicted objects. (ray-project#2033) Fix compilation error for RAY_USE_NEW_GCS with latest clang. (ray-project#2086) [xray] Corrects Error Handling During Push and Pull. (ray-project#2059) [xray] Sophisticated task dependency management (ray-project#2035) Support calling positional arguments by keyword (fix ray-project#998) (ray-project#2081) [DataFrame] Improve performance of iteration methods (ray-project#2026) [DataFrame] Implement to_csv (ray-project#2014) [xray] Lineage cache only requests notifications about remote parent tasks (ray-project#2066) [rllib] Add magic methods for rollouts (ray-project#2024) [DataFrame] Allows DataFrame constructor to take in another DataFrame (ray-project#2072) Pin Pandas version for Travis to 0.22 (ray-project#2075) Fix python linting (ray-project#2076) [xray] Fix GCS table prefixes (ray-project#2065) Some tests for _submit API. (ray-project#2062) [rllib] Queue lib for python 2.7 (ray-project#2057) [autoscaler] Remove faulty assert that breaks during downscaling, pull configs from env (ray-project#2006) ...

AmplabJenkins · 2018-05-18T22:56:47Z

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

AmplabJenkins · 2018-05-19T01:17:16Z

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

alok · 2018-05-19T21:19:34Z

@richardliaw @robertnishihara This passed tests and is ready for merge.

richardliaw · 2018-05-19T22:22:12Z

What does pyupgrade do?

alok · 2018-05-19T22:31:31Z

The examples in the README make it pretty clear:

https://github.com/asottile/pyupgrade

richardliaw · 2018-05-19T23:28:20Z

I guess it's not clear to me why we want it, and it seems to be a relatively arbitrary library (with only 22 stars and 3 forks?), so including this in the codebase doesn't seem like the best idea.

Are there rules that flake8-comprehensions + other linting things we have in place are lacking?

alok · 2018-05-19T23:29:38Z

It just relieves some of the burden of actually following flake8-comprehensions extra lint rules by automatically applying a large subset of them.

As for its low adoption, that doesn't seem too worrying to me since we could always stop using it and apply the flake8 changes manually if pyupgrade broke one day.

richardliaw · 2018-05-19T23:58:37Z

OK if you don't mind I would prefer to keep it out of the code as users can just do this individually.

In case users want to still use it on their own, the upgrade-syn.sh script was left in the `.travis` dir.

alok · 2018-05-20T00:02:31Z

Removed it. I left the script to run it in case users want to do it on their own, but it isn't installed or run automatically.

richardliaw · 2018-05-20T00:03:10Z

Thanks!

AmplabJenkins · 2018-05-20T01:10:40Z

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

alok · 2018-05-20T22:47:57Z

@richardliaw This is (actually) ready for merge now. Passed lint and applied the changes you requested.

pcmoritz

+1, thanks for doing this!

* master: (24 commits) Performance fix (ray-project#2110) Use flake8-comprehensions (ray-project#1976) Improve error message printing and suppression. (ray-project#2104) [rllib] [doc] Broken link in ddpg doc YAPF, take 3 (ray-project#2098) [rllib] rename async -> _async (ray-project#2097) fix unused lambda capture (ray-project#2102) [xray] Use pubsub instead of timeout for ObjectManager Pull. (ray-project#2079) [DataFrame] Update _inherit_docstrings (ray-project#2085) [JavaWorker] Changes to the build system for support java worker (ray-project#2092) [xray] Fix bug in updating actor execution dependencies (ray-project#2064) [DataFrame] Refactor __delitem__ (ray-project#2080) [xray] Better error messaging when pulling from self. (ray-project#2068) Use source code in hash where possible (fix ray-project#2089) (ray-project#2090) Functions for flushing done tasks and evicted objects. (ray-project#2033) Fix compilation error for RAY_USE_NEW_GCS with latest clang. (ray-project#2086) [xray] Corrects Error Handling During Push and Pull. (ray-project#2059) [xray] Sophisticated task dependency management (ray-project#2035) Support calling positional arguments by keyword (fix ray-project#998) (ray-project#2081) [DataFrame] Improve performance of iteration methods (ray-project#2026) ...

* fix-a3c-torch: (37 commits) Add missing channel major Use correct filter size Add TODO Fix shape errors fmt Performance fix (ray-project#2110) Use flake8-comprehensions (ray-project#1976) Improve error message printing and suppression. (ray-project#2104) [rllib] [doc] Broken link in ddpg doc YAPF, take 3 (ray-project#2098) [rllib] rename async -> _async (ray-project#2097) fix unused lambda capture (ray-project#2102) [xray] Use pubsub instead of timeout for ObjectManager Pull. (ray-project#2079) [DataFrame] Update _inherit_docstrings (ray-project#2085) [JavaWorker] Changes to the build system for support java worker (ray-project#2092) [xray] Fix bug in updating actor execution dependencies (ray-project#2064) [DataFrame] Refactor __delitem__ (ray-project#2080) [xray] Better error messaging when pulling from self. (ray-project#2068) Use source code in hash where possible (fix ray-project#2089) (ray-project#2090) Functions for flushing done tasks and evicted objects. (ray-project#2033) ...

alok added 3 commits May 1, 2018 11:19

Add flake8 to Travis

1111e64

Add flake8-comprehensions

55ecc2f

[flake8 plugin](https://github.com/adamchainz/flake8-comprehensions) that checks for useless constructions.

Use generators instead of lists where appropriate

8b3a641

A lot of the builtins can take in generators instead of lists. This commit applies `flake8-comprehensions` to find them.

alok added 3 commits May 1, 2018 14:22

Fix lint error

43588cc

Fix some string formatting

d2ca1f8

The rest can be fixed in another PR

Fix compound literals syntax

9e3cf43

This should probably be merged after ray-project#1963.

alok added 5 commits May 11, 2018 09:25

Merge branch 'master' into comps

23ee644

* master: [xray] Fix UniqueID hashing for object and task IDs. (ray-project#2017) [DataFrame] Fixing bugs in groupby (ray-project#2031) [DataFrame] Fixes dropna subset bug (ray-project#2018) [DataFrame] Implement where (ray-project#1989)

dict() -> {}

f96c910

Use dict literal syntax

52713b9

dict(...) -> {...}

Rewrite nested dicts

89e251f

Fix hanging indent

165d007

alok added 3 commits May 14, 2018 19:38

Add missing import

062007e

Add missing quote

e4bf8d3

fmt

d220ab9

Add missing whitespace

406af2c

alok added 2 commits May 15, 2018 00:48

rm duplicate pip install

63f2190

This is already installed in another file.

richardliaw requested changes May 15, 2018

View reviewed changes

Fix indent

beb9667

richardliaw mentioned this pull request May 15, 2018

[rllib] Catalog should not have dicts as default args #2063

Closed

richardliaw self-assigned this May 15, 2018

move merge_dicts into utils

7e3f7db

richardliaw approved these changes May 16, 2018

View reviewed changes

alok added 2 commits May 18, 2018 21:41

Bring up to date with master

239992e

Add automatic syntax upgrade

52fb515

rm pyupgrade

bfe93db

In case users want to still use it on their own, the upgrade-syn.sh script was left in the `.travis` dir.

pcmoritz approved these changes May 20, 2018

View reviewed changes

pcmoritz merged commit f795173 into ray-project:master May 20, 2018

alok deleted the comps branch May 21, 2018 20:16

Conversation

alok commented May 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What do these changes do?

Related issue number

Uh oh!

AmplabJenkins commented May 1, 2018

Uh oh!

AmplabJenkins commented May 1, 2018

Uh oh!

AmplabJenkins commented May 9, 2018

Uh oh!

pcmoritz commented May 11, 2018

Uh oh!

AmplabJenkins commented May 11, 2018

Uh oh!

richardliaw commented May 14, 2018

Uh oh!

AmplabJenkins commented May 14, 2018

Uh oh!

AmplabJenkins commented May 15, 2018

Uh oh!

AmplabJenkins commented May 15, 2018

Uh oh!

AmplabJenkins commented May 15, 2018

Uh oh!

richardliaw May 15, 2018

Choose a reason for hiding this comment

Uh oh!

alok May 15, 2018

Choose a reason for hiding this comment

Uh oh!

richardliaw May 15, 2018

Choose a reason for hiding this comment

Uh oh!

alok May 15, 2018

Choose a reason for hiding this comment

Uh oh!

richardliaw May 15, 2018

Choose a reason for hiding this comment

Uh oh!

alok May 15, 2018

Choose a reason for hiding this comment

Uh oh!

richardliaw May 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robertnishihara commented May 15, 2018

Uh oh!

alok commented May 15, 2018

Uh oh!

richardliaw commented May 15, 2018 via email

Uh oh!

alok commented May 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AmplabJenkins commented May 16, 2018

Uh oh!

AmplabJenkins commented May 18, 2018

Uh oh!

AmplabJenkins commented May 19, 2018

Uh oh!

alok commented May 19, 2018

Uh oh!

richardliaw commented May 19, 2018

Uh oh!

alok commented May 19, 2018

Uh oh!

richardliaw commented May 19, 2018

Uh oh!

alok commented May 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

richardliaw commented May 19, 2018

Uh oh!

alok commented May 20, 2018

Uh oh!

richardliaw commented May 20, 2018

Uh oh!

alok commented May 1, 2018 •

edited

Loading

richardliaw May 15, 2018 •

edited

Loading

alok commented May 16, 2018 •

edited

Loading

alok commented May 19, 2018 •

edited

Loading