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

Clean up syntax for supported Python versions. #1963

Merged
merged 5 commits into from
May 3, 2018

Conversation

alok
Copy link
Contributor

@alok alok commented Apr 28, 2018

What do these changes do?

Replaces syntax with better syntax.

The dict/set/tuple literals (curly braces) for comprehensions and instantiation is
clearer and faster than writing unidiomatic calls to each of those functions.

Related issue number

No.

alok added 2 commits April 28, 2018 01:04
Ran code through [pyupgrade](https://github.com/asottile/pyupgrade). This is
supported in every Python version 2.7+.
No need to specify 0,1.. if paramters are passed in order.
@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/5103/
Test PASSed.

@@ -572,8 +572,8 @@ def extract_code_globals(cls, co):
# PyPy "builtin-code" object
out_names = set()
else:
out_names = set(names[oparg]
for op, oparg in _walk_global_ops(co))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's not make any changes to this file as it is directly copied from the cloudpickle repository.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Put in a PR w/cloudpickle to change that though.

@@ -260,7 +260,7 @@ def __repr__(self):
# The split here is so that we don't repr pandas row lengths.
result = self._repr_helper_()
final_result = repr(result).rsplit("\n\n", maxsplit=1)[0] + \
"\n\n[{0} rows x {1} columns]".format(len(self.index),
"\n\n[{} rows x {} columns]".format(len(self.index),
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we do this change (and others) we'll need to fix the indentation of the next line

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'm fine leaving this as is then, though I'd prefer that there be a git hook to require all code to be run through auto-formatters. It'd unify things like the use of certain syntax, indent, and quote styles.

@robertnishihara
Copy link
Collaborator

@devin-petersohn can you take a look at the changes to the dataframes files and see if these make sense to you? If you prefer we can make these changes only for non-dataframes files.

@devin-petersohn
Copy link
Member

@robertnishihara @alok These changes are fine for the dataframes files. Thanks!

alok added 2 commits April 29, 2018 22:48
Drop use of set literal until cloudpickle uses it.
@alok
Copy link
Contributor Author

alok commented Apr 30, 2018

I think those are both of the necessary changes.

@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/5127/
Test PASSed.

@robertnishihara
Copy link
Collaborator

Looks good, thanks! There are still a couple linting errors at https://travis-ci.org/ray-project/ray/jobs/372891975. Could you fix those?

We need to set up a git pre-push hook to automatically run this stuff.
@alok
Copy link
Contributor Author

alok commented May 1, 2018

I think that's all the lint errors taken care of.

@robertnishihara
Copy link
Collaborator

Thanks @alok, yapf seems to be complaining (though unfortunately it doesn't list the diff).

$ .travis/yapf.sh
~/build/ray-project/ray/test ~/build/ray-project/ray
~/build/ray-project/ray
~/build/ray-project/ray/python ~/build/ray-project/ray
~/build/ray-project/ray
Reformatted staged files. Please review and stage the changes.
Files updated:
  python/ray/worker.py
  test/actor_test.py
  test/stress_tests.py

@alok
Copy link
Contributor Author

alok commented May 1, 2018

@robertnishihara I ran yapf locally with the same flags and it gave no changes. Travis says the build ran 19 hours ago, so maybe it hasn't picked up the changes yet?

alok added a commit to alok/ray that referenced this pull request May 1, 2018
This should probably be merged after ray-project#1963.
@pcmoritz
Copy link
Contributor

pcmoritz commented May 2, 2018

There are still linting errors, can you have another look?

@robertnishihara
Copy link
Collaborator

Maybe it's a version issue with yapf?

@alok
Copy link
Contributor Author

alok commented May 3, 2018

The cloudpickle changes were just merged in, so we can pick them up next time we update those files.

@alok alok closed this May 3, 2018
@alok alok reopened this May 3, 2018
@alok
Copy link
Contributor Author

alok commented May 3, 2018

@pcmoritz
Copy link
Contributor

pcmoritz commented May 3, 2018

Jenkins test this please

@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/5171/
Test PASSed.

@pcmoritz pcmoritz merged commit cdf94c1 into ray-project:master May 3, 2018
@alok alok mentioned this pull request May 6, 2018
alok added a commit to alok/ray that referenced this pull request May 6, 2018
* magic-methods:
  fmt
  Fix IndentationError
  Write magic methods for SampleBatch/PartialRollout
  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)
  add pthread linking (ray-project#1986)
  [DataFrame] Add layer of abstraction to allow OID instantiation (ray-project#1984)
  [DataFrame] Fix blocking issue on _IndexMetadata passing (ray-project#1965)
alok added a commit to alok/ray that referenced this pull request May 8, 2018
* master: (21 commits)
  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)
  add pthread linking (ray-project#1986)
  [DataFrame] Add layer of abstraction to allow OID instantiation (ray-project#1984)
  ...
alok added a commit to alok/ray that referenced this pull request May 9, 2018
* 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)
  ...
pcmoritz pushed a commit that referenced this pull request May 20, 2018
* Add flake8 to Travis

* Add flake8-comprehensions

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

* Use generators instead of lists where appropriate

A lot of the builtins can take in generators instead of lists.

This commit applies `flake8-comprehensions` to find them.

* Fix lint error

* Fix some string formatting

The rest can be fixed in another PR

* Fix compound literals syntax

This should probably be merged after #1963.

* dict() -> {}

* Use dict literal syntax

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

* Rewrite nested dicts

* Fix hanging indent

* Add missing import

* Add missing quote

* fmt

* Add missing whitespace

* rm duplicate pip install

This is already installed in another file.

* Fix indent

* move `merge_dicts` into utils

* Bring up to date with `master`

* Add automatic syntax upgrade

* rm pyupgrade

In case users want to still use it on their own, the upgrade-syn.sh script was
left in the `.travis` dir.
@alok alok deleted the upgrade-syn branch May 21, 2018 10:03
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.

5 participants