Skip to content

Better image pyramid#109

Merged
fcollman merged 20 commits intodevelopfrom
better_image_pyramid
Jul 11, 2018
Merged

Better image pyramid#109
fcollman merged 20 commits intodevelopfrom
better_image_pyramid

Conversation

@fcollman
Copy link
Copy Markdown
Member

I'd like to reopen this discussion about a better way to handle the image pyramid interface.

changed image pyramid interface to be more straightforward, and changed tests where necessary

bumped version to indicate backward compatability break

wip

fixed auto sorting
fixed s3 bucket read

added new requirements

removed auto-import of image_open

making urlparse python3 compatible

fixing python3 imports more

another import fix
@fcollman fcollman requested a review from RussTorres June 13, 2018 20:26
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 13, 2018

Codecov Report

Merging #109 into develop will increase coverage by 1.04%.
The diff coverage is 97.75%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #109      +/-   ##
===========================================
+ Coverage    93.91%   94.96%   +1.04%     
===========================================
  Files           16       17       +1     
  Lines         1694     2047     +353     
===========================================
+ Hits          1591     1944     +353     
  Misses         103      103
Impacted Files Coverage Δ
renderapi/stack.py 95.2% <ø> (-0.07%) ⬇️
renderapi/image_pyramid.py 100% <100%> (+20%) ⬆️
renderapi/channel.py 100% <100%> (ø) ⬆️
renderapi/tilespec.py 95.68% <81.81%> (+1.23%) ⬆️
renderapi/external/processpools/stdlib_pool.py 100% <0%> (ø)
renderapi/transform.py 94.27% <0%> (+0.13%) ⬆️
renderapi/utils.py 97.26% <0%> (+1.8%) ⬆️
renderapi/client.py 91.8% <0%> (+2.5%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4d9a8e5...3b2fd00. Read the comment docs.

return "{}".format(level)

def to_dict(self):
return {k: v.to_dict() for k, v in self.items()}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we want these to return empty mipmaplevels e.g.

{
  "0": {},
  "1": {
    "imageUrl": "file:///path/to/img",
    "maskUrl": null
},
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

hmm... I don't think so. Is there a reason we should?

def to_dict(self):
return {k: v.to_dict() for k, v in self.items()}

@staticmethod
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Any reason not to make this a classmethod? Might be tempted to subclass this at some point.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

no reason. did you mean to_dict or line 157?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I meant the from_dict method so that a subclass will return an instance of itself rather than ImagePyramid.

"""

def __init__(self, level, imageUrl=None, maskUrl=None):
logger.warning(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

should we use warnings.DeprecationWarning for this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yes... will change

d.update({'maskUrl': self.maskUrl})
return d

def __getitem__(self, key):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

should this also support assignment with a setitem? The current implementation does not support

ip[0]['imageUrl'] = "file:///path/to/image"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

oh i don't see why not... might actually make it more backwards compatible.

@fcollman fcollman merged commit bb301bf into develop Jul 11, 2018
@fcollman fcollman deleted the better_image_pyramid branch August 17, 2018 14:56
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.

2 participants