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

Don't use str.find #154

Merged
merged 2 commits into from Dec 8, 2019
Merged

Don't use str.find #154

merged 2 commits into from Dec 8, 2019

Conversation

domoritz
Copy link
Contributor

It doesn't work for unicodes in Python 2.

Copy link
Owner

@berkerpeksag berkerpeksag left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Could you share a reproducer? It would be nice to add a test case.

@domoritz
Copy link
Contributor Author

I ran into this issue when I created an ast node in Python 2 but set the name to a unicode.

import astor
import ast
astor.to_source(ast.Name(id=u'copy', ctx=ast.Load()))

@berkerpeksag
Copy link
Owner

Thanks! PR looks good to me. I'll convert your reproducer to a test case (feel free to do that if you have some spare time) and merge it.

Test failure seems unrelated as I can see it fails in other PRs:

======================================================================
FAIL: test_positional_only_arguments (tests.test_code_gen.CodegenTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/berkerpeksag/astor/tests/test_code_gen.py", line 180, in test_positional_only_arguments
    self.assertSrcRoundtrips(source)
  File "/home/travis/build/berkerpeksag/astor/tests/test_code_gen.py", line 72, in assertSrcRoundtrips
    self.assertEqual(self.to_source(ast.parse(srctxt)).rstrip(), srctxt)
AssertionError: 'def [42 chars]s\n\n\ndef test(a=3, b=4, /, c=7):\n    pass\n[40 chars]pass' != 'def [42 chars]s\n\ndef test(a=3, b=4, /, c=7):\n    pass\n\n[36 chars]pass'
  def test(a, b, /, c, *, d, **kwargs):
      pass
- 
  
  def test(a=3, b=4, /, c=7):
      pass
  
- 
  def test(a, b=4, /, c=8, d=9):
      pass

domoritz and others added 2 commits December 8, 2019 15:02
It doesn't work for unicodes in Python 2.
Copy link
Owner

@berkerpeksag berkerpeksag left a comment

Choose a reason for hiding this comment

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

LGTM! I've just added a test case and updated changelog.

@berkerpeksag berkerpeksag merged commit 0b3cbce into berkerpeksag:master Dec 8, 2019
@berkerpeksag
Copy link
Owner

Thank you and sorry for my late response!

isidentical pushed a commit to isidentical/astor that referenced this pull request Dec 9, 2019
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.

None yet

2 participants