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

Make it possible to specify an origin (e.g. PARENT) with e.g. im[bbox] #272

Merged
merged 1 commit into from Aug 12, 2017

Conversation

RobertLuptonTheGood
Copy link
Member

That is, when using the syntactic sugar which makes using bboxes easier,
allow an extra argument such as
im[bbox, afwImage.PARENT]
or
im[1000:1010, -1:, afwImage.PARENT]

In the second case we'll use PARENT coordinates when interpreting the
slices (i.e. the "1000" includes x0)

Copy link
Contributor

@SimonKrughoff SimonKrughoff left a comment

Choose a reason for hiding this comment

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

This looks fine. I have a couple of comments, but I'm sure I'm just confused.


try:
_origin = imageSlice[-1]
except TypeError:
Copy link
Contributor

Choose a reason for hiding this comment

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

Where would you get a type error here. It would happen if imageSlice isn't listy, but then you've got bigger problems. Are you thinking about the case where imageSlice is a slice object?

Copy link
Member Author

Choose a reason for hiding this comment

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

You can get bbox or bbox, origin as well as list-like things

I don't guarantee that I have the minimal logic to cover all the cases, so I should take another look.

Copy link
Contributor

Choose a reason for hiding this comment

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

I hadn't appreciated that imageSlice could be a bbox.


if isinstance(_origin, ImageOrigin):
origin = _origin
imageSlice = imageSlice[0] if len(imageSlice) <= 2 else imageSlice[:-1]
Copy link
Contributor

Choose a reason for hiding this comment

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

I must be missing something here too. It seems like imageSlice would only ever be 2 or 3 elements long. If it's 3, it has an origin, if not you want both the elements. Why only pass on the first element?

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is that you can get passed lots of different things:

  • bbox
  • bbox, origin
  • :
  • :, origin
  • n:m, :
  • n:m, :, origin
  • n:m, i:j
  • n:m, i:j, origin

You don't want to convert bbox to [bbox]

Copy link
Contributor

Choose a reason for hiding this comment

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

I see now. It may be worth adding a few more of these as examples in the docstring.

That is, when using the syntactic sugar which makes using bboxes easier,
allow an extra argument such as
    im[bbox, afwImage.PARENT]
or
	 im[1000:1010, -1:, afwImage.PARENT]

In the second case we'll use PARENT coordinates when interpreting the
slices (i.e. the "1000" includes x0)
@RobertLuptonTheGood RobertLuptonTheGood merged commit d2c76a8 into master Aug 12, 2017
@ktlim ktlim deleted the tickets/DM-11579 branch August 25, 2018 06:44
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