Skip to content

add a way to get image size without loading it#111

Merged
bjarthur merged 1 commit intoJuliaIO:masterfrom
bjarthur:bja/size
May 14, 2018
Merged

add a way to get image size without loading it#111
bjarthur merged 1 commit intoJuliaIO:masterfrom
bjarthur:bja/size

Conversation

@bjarthur
Copy link
Copy Markdown
Member

@bjarthur bjarthur commented Jan 31, 2018 •
edited
Loading

ImageMagick.size("filename.tif") returns a tuple of dimensions.

my only reservation is calling it size, which might not dispatch properly if just given a String.

also, is there a way to integrate this into the FileIO API? see JuliaIO/FileIO.jl#173

timholy
timholy reviewed Feb 1, 2018
View reviewed changes
Comment thread src/ImageMagick.jl Outdated
The returned value is (X,Y[,Z]), not (R,C), so the first two dimensions
of the corresponding Array returned by `load` are reversed.
"""
function size(file)
Copy link
Copy Markdown
Member

@timholy timholy Feb 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I'm puzzled about how this passed without renaming all other internal size calls to Base.size.

EDIT: now I see there's an import Base: size in libmagickwand.jl. Mystery solved. But more problematic, too.

@timholy
Copy link
Copy Markdown
Member

timholy commented Feb 1, 2018

The idea here is great...I just share your unease about calling it size, especially since this is rampant type-piracy. Maybe if we qualify all the internal calls to Base.size and then avoid extending the Base function?

@bjarthur
Copy link
Copy Markdown
Member Author

bjarthur commented Feb 1, 2018

how about imsize? would match (some of the) naming convention in Images, and avoid type-piracy and Base.size altogether.

should probably decide about how this might integrate with FileIO before merging though. would be nice if there were a metadata function in FileIO, which returned a sub-package defined type that contained, in ImageMagick's case, height, width, depth, eltype, etc.

@timholy
Copy link
Copy Markdown
Member

timholy commented Feb 1, 2018

I like the metadata proposal!

@bjarthur bjarthur force-pushed the bja/size branch 2 times, most recently from 1cb7640 to b0a1373 Compare April 11, 2018 14:24
@bjarthur bjarthur mentioned this pull request Apr 11, 2018
Merged
@bjarthur bjarthur mentioned this pull request Apr 11, 2018
Merged
@bjarthur
Copy link
Copy Markdown
Member Author

bjarthur commented May 5, 2018

any objections to merging this? see also JuliaIO/FileIO.jl#178 and JuliaImages/Images.jl#714

@bjarthur bjarthur merged commit 8f8bb8d into JuliaIO:master May 14, 2018
@bjarthur bjarthur deleted the bja/size branch May 14, 2018 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@timholy timholy timholy left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bjarthur @timholy