Skip to content

Add unreadable_python_inputs#205

Closed
ritwik12 wants to merge 4 commits intogoogle:masterfrom
ritwik12:patch-1
Closed

Add unreadable_python_inputs#205
ritwik12 wants to merge 4 commits intogoogle:masterfrom
ritwik12:patch-1

Conversation

@ritwik12
Copy link
Copy Markdown
Contributor

Few Changes in try block in continuation with #204

Copy link
Copy Markdown
Contributor

@rchen152 rchen152 left a comment

Choose a reason for hiding this comment

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

Thanks! Two minor comments.

@rchen152
Copy link
Copy Markdown
Contributor

@ritwik12 Thank you so much for your patience! Since Peter's requested change isn't directly related to your edits, I went ahead and tweaked the logging.critical(...) line again. I'll approve this PR once the tests have rerun, but please don't merge it yet - there's a bit of extra work I have to do to ensure that the Google-internal copy of the pytype code stays in sync.

@pludemann Thanks for the suggestion. Since the unparseable files are listed one per line, condensing the header message wouldn't actually put the whole error message on one line, so I've put the \n back for now. I'd prefer to not increase the scope of this PR, since it's already a follow-on to another one.

@kamahen
Copy link
Copy Markdown
Contributor

kamahen commented Dec 17, 2018

@rchen152 -- FWIW, I wrote a multi-line logging function that basically did msg.splitlines() and then called the appropriate logging function for each line. I know I did this for google3 C++ (as a macro) about 10 years ago but can't remember if I also did it for Python. For this particular situation, you could, of course do something like this:

for err_line in ('Cannot parse input files:\n' + str(e)).splitlines():
    logging.error(err_line)
logging.critical('Cannot proceed due to parsing failure')

@rchen152 rchen152 closed this in 9ad3b33 Dec 17, 2018
@rchen152
Copy link
Copy Markdown
Contributor

This change has been merged (9ad3b33) :)

@ritwik12
Copy link
Copy Markdown
Contributor Author

@rchen152 Your welcome and thanks for the edits.

@ritwik12
Copy link
Copy Markdown
Contributor Author

@rchen152 Just a little doubt, how does it shows "closed" on this PR if it is merged?

@rchen152
Copy link
Copy Markdown
Contributor

That's a side effect of how we keep our two copies of the pytype code base in sync - I imported your PR into Google, re-exported it (#210), then used the export PR to close this one. If you check the commits on the master branch, you should find "Add unreadable_python_inputs (#205)" on December 17.

@ritwik12
Copy link
Copy Markdown
Contributor Author

ritwik12 commented Dec 18, 2018

@rchen152 Yes, It's great. Got it.

@ritwik12
Copy link
Copy Markdown
Contributor Author

@rchen152 @kamahen How can one join https://github.com/google organization?

@rchen152
Copy link
Copy Markdown
Contributor

I believe you have to be a current Googler working on an open-source project to join that organization.

@ritwik12
Copy link
Copy Markdown
Contributor Author

@rchen152 Googler meaning a Google employee? Right

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.

4 participants