-
Notifications
You must be signed in to change notification settings - Fork 492
Replace sparkdl's ImageSchema with Spark2.3's version #85
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
Merged
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
adc132d
Initial implementation of replacement of sparkdl's version of ImageSc…
tomasatdatabricks fd9a38f
Adressed Sid's and Sue Ann's review comments.
tomasatdatabricks 537d1b1
Hack: Per Tim's suggestion I copied Spark2.3's ImageSchema files into…
tomasatdatabricks 5516b50
Minor update.
tomasatdatabricks 327ddc8
Addressed some review comments:
tomasatdatabricks a5f2ff1
Added comments explaining why reveresing color channels is necessary …
tomasatdatabricks 0ea3761
Per review comments and offline discusion, removed reordering of outp…
tomasatdatabricks 936838a
Some cosmetic changes address review comments.
tomasatdatabricks 1ab34bf
Addressed review comments and follow up offline discussion: Added par…
tomasatdatabricks c2e803b
Per reviewer's request, moved copy-pasted ImageSchema file from pyspa…
tomasatdatabricks a3f4d08
Cosmetic changes addressing review comments
tomasatdatabricks 421c924
Added convertor to check channel order parameter.
tomasatdatabricks 1a117b4
minor changes addressing review comments
tomasatdatabricks 005fd61
Ran autopep8 on imageIO.py
tomasatdatabricks 10c182c
Ran autopep8 on all sparkdl and test folders
tomasatdatabricks def1e0e
Minor test fix: added missing cls arg in named_image_test's setup
tomasatdatabricks 5ef9a6b
Minor fix, added extra channel order to error message
tomasatdatabricks File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,5 @@ | ||
| // You may use this file to add plugin dependencies for sbt. | ||
| resolvers += "Spark Packages repo" at "https://dl.bintray.com/spark-packages/maven/" | ||
|
|
||
| addSbtPlugin("org.spark-packages" %% "sbt-spark-package" % "0.2.5") | ||
|
|
||
| // scalacOptions in (Compile,doc) := Seq("-groups", "-implicits") | ||
|
|
||
| addSbtPlugin("org.scoverage" % "sbt-scoverage" % "1.5.0") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
in the new schema, are there legitimate types that have float64 (or any other dtypes) as img_dtype?
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.
AFAIK the schema does not specify types. It only specifies a field with OpenCv type number. There are open CV types which have float64. Technically the schema includes openCvTypes map with only a subset of types, however we already need types outside of this subset (Tf produced images are stored as float32)
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.
So does ImageSchema support OpenCV types that have float64? If so, should we support them here?
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.
Currently as far as I know there is no way how you can get float64 image.
ImageSchema as a data format supports it in that it has a mode field which is supposed to have OpenCV type in it and there are OpenCV types with float64. However, it is not listed in the list of openCV types in their scala code (and neither are any float32 which we need) and as it stands now, readImages can only ever produce images stored in unsigned bytes (both scala an PIL version) so one of CV_8U* formats. We also need the float32 formats since thats' what we return when returning images from TF so I added those to our python side.
The python code from image schema can only handle unsigned byte images, thats why I use our own version in imageIO (imageArrayToStruct and imageStructToArray).
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.
From offline discussion: The ImageSchema utilities in Spark only support uint8 types. Ideally float32 types would also be supported natively in Spark so we don't have to have special logic in this package to handle it. We'll create a Jira in Spark for that and try to address it there.
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.
Do we have a Jira for this already? If so, could you link from here?
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.
yes we do. https://issues.apache.org/jira/browse/SPARK-22730
You mean you want it in the code? That would probably go to imageIO, I'll put it there