parse url to set parameters and return parameters as dict

[ghukill]

Not sure at all if this is in scope for the IIIFImageClient, but thought it was worth throwing out there.

Would be two additions:

• ability to provide and parse a URL to set the api_endpoint, image_id, region, size, rotation, quality, and format of the client instance
• return a dictionary that parses the more complex parameters of region, size, and rotation

We have a use case where we want to determine the height and/or width that a URL is requesting, and whether or not it's mirrored. These two additions make that pretty easy given a URL.

e.g.

from piffle.iiif import IIIFImageClient
ic = IIIFImageClient()

ic.parse_from_url('http://localhost/loris/foo:bar/full/full/0/default.jpg')
In [5]: ic.api_params_as_dict()
Out[5]: 
{'region': {'full': True,
  'h': None,
  'pct': False,
  'w': None,
  'x': None,
  'y': None},
 'rotation': {'degrees': 0, 'mirrored': False},
 'size': {'exact': False, 'full': True, 'h': None, 'pct': False, 'w': None}}

# with more parameters
ic.parse_from_url('http://localhost/loris/foo:bar/2560,2560,256,256/256,/!90/default.jpg')
In [11]: ic.api_params_as_dict()
Out[11]: 
{'region': {'full': False,
  'h': '256',
  'pct': False,
  'w': '256',
  'x': '2560',
  'y': '2560'},
 'rotation': {'degrees': 90, 'mirrored': True},
 'size': {'exact': False, 'full': False, 'h': None, 'pct': False, 'w': 256}}

Either way, thanks for breaking this into it's own package! Mighty handy.

[coveralls]

Coverage decreased (-37.6%) to 57.407% when pulling bdfb585 on WSULib:parseurl into 9821b64 on emory-lits-labs:master.

[coveralls]

Coverage decreased (-4.3%) to 90.741% when pulling 3202440 on WSULib:parseurl into 9821b64 on emory-lits-labs:master.

[rlskoeser]

This seems pretty slick - it lets you can initialize an image client instance from a url? I think maybe it would be a bit cleaner to make it a classmethod and return a new instance. And what do you think of calling it init_from_url instead of parse?

[ghukill]

All sound like excellent ideas, thanks for the feedback. Will give those changes a go and update this pull request.

[rlskoeser]

All the updates look good. Given that the last commit is "pre-tests" is it fair to assume more is coming on the unit tests?

Also, might be good to declare and use a custom exception class, just to make it easier to catch and identify.

[coveralls]

Coverage decreased (-8.4%) to 86.555% when pulling 7c3afeb on WSULib:parseurl into 9821b64 on emory-lits-labs:master.

[coveralls]

Coverage decreased (-8.1%) to 86.885% when pulling 15e0e32 on WSULib:parseurl into 9821b64 on emory-lits-labs:master.

[coveralls]

Coverage decreased (-8.1%) to 86.885% when pulling 111f05d on WSULib:parseurl into 9821b64 on emory-lits-labs:master.

[ghukill]

Alrighty, think I've addressed most of your suggestions:

• renamed the method to init_from_url(), and has it return new class instance
• error handling for malformed urls, raises custom InitURLError exception
• used dict_opts for aggregated parsed parameters
• sub-methods are public (as they should have been in the first place)
• tightened up value types that are returned from parsed parameters, returning float when percentage is used per the spec
• added tests that assert successful init from urls, exceptions catching bag ones, and for a complex image request url, the sub-methods for parsing parameters are returning the expected dictionaries

Thanks again for taking the time to look over the initial PR and provide feedback, it was pretty rough around the edges the first go.

[rlskoeser]

Updates look good. I'll try to get it merged in and maybe make another release soon.

[ghukill]

Awesome, thanks.

[rlskoeser]

@ghukill I'm doing some clean up and extending the unit tests based on your additions, and I'm wondering if it would make more sense to store the options internally as dictionaries rather than strings. That would mean options provided at init would be parsed and checked (which they currently aren't), and seems like it might be a more logical approach in general. Thoughts?

[ghukill]

That's a great idea. I had originally envisioned methods like region_as_dict, size_as_dict, etc. to parse dictionaries from strings because they were stored as strings in the original init. But yeah, reworking init to store the image request parameters as dictionaries right from the get-go, that makes sense. Then you can check the values at that time, and know right away if it's a valid IIIF url. Let me know if I can help at all.

[rlskoeser]

I'm going to go ahead and close this pull request, since the code is merged into develop and documented in the change log. Thanks for the contribution!