8000 Resolve tools/testein.py failure on OS X by jhamrick · Pull Request #120 · tkf/emacs-ipython-notebook · GitHub
[go: up one dir, main page]
Skip to content

Resolve tools/testein.py failure on OS X #120

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 29, 2013
Merged

Resolve tools/testein.py failure on OS X #120

merged 1 commit into from
Jun 29, 2013

Conversation

jhamrick
Copy link
Contributor

This should fix #119.

The arguments to ps do not translate nicely between versions, unfortunately, so it won't suffice to just have a single command. Instead, I changed it to try to run the GNU version first, and if that fails, then try the version that should work on OS X.

@tkf
Copy link
8000
Owner
tkf commented Jun 28, 2013

You don't need to use gnu options. Linux ps should have unix options. Let's keep the logic minimum, even if it requires a loop. This way, it should run on Linux if it runs on Mac. But It is sad that BSD ps does not have --ppid equivalent...

@jhamrick
Copy link
Contributor Author

The issue I was trying to get around was that there is also no equivalent option for --no-headers. But actually, I realize that wouldn't matter if it's going to retrieve ppid and do the filtering in Python rather than with ps. I think then the correct command would be:

ps -e -o ppid,pid,command

And then check in the Python loop that ppid matches the target parent process.

@jhamrick
Copy link
Contributor Author

Still failing with the invalid literal for int() with base 10: '' error. I think there are probably two spaces between the two pids, and so this is basically what's happening:

In [1]: "3423  2348  foo".split(' ', 2)
Out[1]: ['3423', '', '2348  foo']

Perhaps it should parse the ppid and pid out with a regex rather than using split? Something like this:

re.findall(r"^([0-9]+)[ ]+([0-9]+)[ ]+(.*)$", line)

@tkf
Copy link
Owner
tkf commented Jun 28, 2013

Let's use None:

In [1]: "3423  2348  foo".split(None, 2)
Out[1]: ['3423', '2348', 'foo']

In [2]: str.split?
Type:       method_descriptor
Base Class: <type 'method_descriptor'>
String Form:<method 'split' of 'str' objects>
Namespace:  Python builtin
Docstring:
S.split([sep [,maxsplit]]) -> list of strings

Return a list of the words in the string S, using sep as the
delimiter string.  If maxsplit is given, at most maxsplit
splits are done. If sep is not specified or is None, any
whitespace string is a separator and empty strings are removed
from the result.

@jhamrick
Copy link
Contributor Author

Oh, good idea, I had forgotten split could take None!

@tkf
Copy link
Owner
tkf commented Jun 28, 2013

BTW, if you have extra power please squash commits into one commit :)

tkf added a commit that referenced this pull request Jun 29, 2013
@tkf
Merge pull request #120 from jhamrick/master
0cc4943 
Resolve tools/testein.py failure on OS X
@tkf tkf merged commit 0cc4943 into tkf:master Jun 29, 2013
@tkf
Copy link
Owner
tkf commented Jun 29, 2013

Merged. Thank you!

BTW, github does not notify when someone push to an existing pull request so please put some comment next time :)

@jhamrick
Copy link
Contributor Author

Ah, sorry, I didn't realize it didn't send a notification. Thanks for letting me know, I'll be sure to comment next time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Invalid arguments to ps in testein.py on OS X
2 participants
@jhamrick @tkf
0