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

Reduce IPython.external.* #1199

Merged
merged 5 commits into from Jan 3, 2012
Merged

Reduce IPython.external.* #1199

merged 5 commits into from Jan 3, 2012

Conversation

tomspur
Copy link
Contributor

@tomspur tomspur commented Dec 22, 2011

Delete IPython.external.guid, as it's not needed anymore and do some cleanups regarding external imports

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()
Copy link
Member

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()).

Copy link
Contributor Author

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...

@minrk
Copy link
Member

minrk commented Jan 1, 2012

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?

@fperez
Copy link
Member

fperez commented Jan 3, 2012

@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.

minrk added a commit that referenced this pull request Jan 3, 2012
* 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.
@minrk minrk merged commit 7ac48f7 into ipython:master Jan 3, 2012
@minrk
Copy link
Member

minrk commented Jan 3, 2012

Sounds good to me, I'll merge then.

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
* 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.
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

3 participants