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
Reduce IPython.external.* #1199
Conversation
This reverts commit a36c8d5, where uuid was replaced with guid. Signed-off-by: Thomas Spura <thomas.spura@gmail.com>
Signed-off-by: Thomas Spura <thomas.spura@gmail.com>
Signed-off-by: Thomas Spura <thomas.spura@gmail.com>
Signed-off-by: Thomas Spura <thomas.spura@gmail.com>
Signed-off-by: Thomas Spura <thomas.spura@gmail.com>
@@ -67,7 +67,7 @@ def execute(self, block, blockID=None): | |||
return Failure(Exception("Block is not compilable")) | |||
|
|||
if(blockID == None): | |||
blockID = guid.generate() | |||
blockID = uuid.uuid4() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is in deathrow so it probably doesn't matter, but guid.generate()
returns a string, and uuids are objects so this should probably be str(uuid.uuid4())
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late answer...
As noted in the commit above, I was just reverting commit a36c8d5, but both should work here as I guess, you compare uuids (no matter if they are stings or real objects).
But as you note in the merge it's untested...
This seems sensible, though I personally would have skipped deleting the currently unused imports of the testing decorators and utilities. @fperez - any reason at all to keep guid around? |
@minrk, none that I can think of... The uuid module appeared in python 2.5, so we had our own copy before. Can and should be nuked. As far as I'm concerned, this can be merged. |
* Removes guid from external, because we now use stdlib uuid. * Remove some miscellaneous unused imports * The only remaining guid usage was in deathrow, and has been replaced with uuids, but is untested.
Sounds good to me, I'll merge then. |
* Removes guid from external, because we now use stdlib uuid. * Remove some miscellaneous unused imports * The only remaining guid usage was in deathrow, and has been replaced with uuids, but is untested.
Delete IPython.external.guid, as it's not needed anymore and do some cleanups regarding external imports