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

Add center option to drawImage command #1053

Merged
merged 4 commits into from
Sep 2, 2019
Merged

Add center option to drawImage command #1053

merged 4 commits into from
Sep 2, 2019

Conversation

rmjarvis
Copy link
Member

Sorry, I lied. One more PR for version 2.2. This one is quick though. It's a feature I've wanted to add for a while now, but I never bothered to make an issue for.

I added center as an optional parameter to the drawImage command. It's kind of just a convenience option, since it doesn't add any real capability that isn't already possible with the offset parameter. But getting a galaxy drawn at the right position with offset, especially when letting GalSim automatically size the image for you, is kind of a pain. An example was in demo9, and it was pretty confusing.

Now a whole bunch of code in demo9.py has gone away, and the draw command changed to

final.drawImage(wcs=local_wcs, center=image_pos)

This makes me happy. :) I didn't propagate this change to anywhere else, but it wouldn't surprise me if there were other places where some code could look significantly simpler using center rather than offset.

@rmjarvis rmjarvis added this to the v2.2 milestone Aug 27, 2019
@rmjarvis rmjarvis changed the title Add center option do drawImage command Add center option to drawImage command Aug 27, 2019
Copy link
Member

@jmeyers314 jmeyers314 left a comment

Choose a reason for hiding this comment

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

Works great. LGTM.

Copy link
Member

@rmandelb rmandelb left a comment

Choose a reason for hiding this comment

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

Very nice! Thanks, Mike!

examples/demo9.py Outdated Show resolved Hide resolved
@@ -1604,14 +1638,15 @@ def drawImage(self, image=None, nx=None, ny=None, bounds=None, scale=None, wcs=N
# Figure out what wcs we are going to use.
wcs = self._determine_wcs(scale, wcs, image)

# Make sure offset is a PositionD
# Make sure offset and center are PositionD if entered as tuples.
Copy link
Member

Choose a reason for hiding this comment

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

I think these convenience functions also work if offset/center are entered as lists or a numpy array? Maybe this documentation could be something like "Make sure offset and center are PositionD, converting as necessary from other formats. If center was passed in as None, it will remain that way after _parse_center is run"?

rmjarvis and others added 2 commits September 2, 2019 12:03
Co-Authored-By: Rachel Mandelbaum <rmandelb@andrew.cmu.edu>
@rmjarvis rmjarvis merged commit f5a6e02 into master Sep 2, 2019
@rmjarvis rmjarvis deleted the draw_center branch September 2, 2019 17:02
@rmjarvis
Copy link
Member Author

rmjarvis commented Sep 2, 2019

Thanks both. Let's plan to release 2.2.0 on Friday. So if you have any systems you want to check to make sure we didn't break installation or anything, please check them this week.

@rmjarvis rmjarvis added the base classes Related to GSObject, Image, or other base GalSim classes label Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
base classes Related to GSObject, Image, or other base GalSim classes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants