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

Thunder integration with OCP #130

Merged
merged 16 commits into from Mar 30, 2015
Merged

Conversation

kunallillaney
Copy link
Contributor

Hey Jeremy,

I have merged the latest branch of thunder and documented my function. In addition, the tests are also fixed. They will not fail if OCP is down.

@kunallillaney
Copy link
Contributor Author

Fixed the bug on our end. All tests now pass for all servers on our end.

@freeman-lab
Copy link
Member

@kunallillaney do you mind merging in the latest changes from master? I now think the problem is that the test output with your changes became longer that Travis's maximum length, but some tooling I just did simplifies the test output dramatically, and should fix it if that's the problem.

@kunallillaney
Copy link
Contributor Author

@freeman-lab Done and all tests pass. And the test output is now much better and readable. Once your are done with the series changes we can also work on the seriesloader for OCP. Thanks.


return data

rdd = self.sc.parallelize (enumerate(urlList), len(urlList)).map(lambda (k, v): (k, read(v)))
Copy link
Member

Choose a reason for hiding this comment

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

remove space after parallelize

@freeman-lab
Copy link
Member

@kunallillaney Thanks for the updates. I left a few comments. There are many deviations from PEP conventions for spacing here, can you open in PyCharm and try to fix them?

Also, after looking at it more, I would rather move the top-level ThunderContext method outside of loadImages and into a separate function called loadImagesOCP. The reason is that there is actually little overlap in the input arguments, there are already a large number of arguments for loadImages, and this is really a very different format than all the other more standard image formats than the ones currently handled by loadImages. Note that this only affects the loadImages method, not the imagesloader.fromOCP which can remain as is.

Can you try making that change?

@kunallillaney
Copy link
Contributor Author

@freeman-lab Did fix all those(and more) PEP errors in PyCharm. Moved OCP out in the context file. Let me know all of this looks good.

@kunallillaney
Copy link
Contributor Author

@freeman-lab Is there any more TODO's on our end?

@@ -133,6 +133,107 @@ def toArray(idxAndBuf):
newDims = tuple(list(dims[:-1]) + [nplanes]) if nplanes else dims
return Images(readerRdd.flatMap(toArray), nrecords=nrecords, dims=newDims, dtype=dtype)

def fromOCP (self, bucketName, resolution, serverName='ocp.me', startIdx=None, stopIdx=None, minBound=None,
maxBound=None):
"""Sets up a new Image object with data to read from OCP
Copy link
Member

Choose a reason for hiding this comment

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

Add definition of OCP

@freeman-lab
Copy link
Member

Just left a couple final requests, and can you merge in from master to figure out the merge conflict? After that we can merge this in! Thanks.

@kunallillaney
Copy link
Contributor Author

@freeman-lab Done and pushed.

@freeman-lab
Copy link
Member

Great! LGTM, thanks for the contribution @kunallillaney !

freeman-lab added a commit that referenced this pull request Mar 30, 2015
Thunder integration with OCP
@freeman-lab freeman-lab merged commit e2a9300 into thunder-project:master Mar 30, 2015
@kunallillaney
Copy link
Contributor Author

Thanks.

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