GitHub PullRequestReviewCommentEvent at 2014-04-11 12:36:59
Repository: GalSim
Actor: rmjarvis
Payload: {"comment":{"url":"https://api.github.com/repos/GalSim-developers/GalSim/pulls/comments/11529813","id":11529813,"diff_hunk":"@@ -274,142 +381,219 @@ def kValue(self, *args, **kwargs):\n         issue (unlike for xValue).\n \n         @param position  The position in k space at which you want the fourier amplitude.\n-        @returns kvalue  The amplitude of the fourier transform at that position.\n+\n+        @returns the amplitude of the fourier transform at that position.\n         \"\"\"\n         kpos = galsim.utilities.parse_pos_args(args,kwargs,'kx','ky')\n         return self.SBProfile.kValue(kpos)\n \n-    def scaleFlux(self, flux_ratio):\n-        \"\"\"Multiply the flux of the object by flux_ratio\n+    def withFlux(self, flux):\n+        \"\"\"Create a version of the current object with a different flux.\n \n-        After this call, the caller's type will be a GSObject.\n-        This means that if the caller was a derived type that had extra methods beyond\n-        those defined in GSObject (e.g. getSigma() for a Gaussian), then these methods\n-        are no longer available.\n+        This function is equivalent to obj.withScaledFlux(flux / obj.getFlux())\n+\n+        It creates a new object that has the same profile as the original, but with the\n+        surface brightness at every location rescaled such that the total flux will be\n+        the given value.\n \n-        @param flux_ratio The factor by which to scale the flux.\n+        @param flux     The new flux for the object.\n+\n+        @returns the object with the new flux\n         \"\"\"\n-        if hasattr(self,'noise'):\n-            self.noise.scaleVariance( flux_ratio**2 )\n-        self.SBProfile.scaleFlux(flux_ratio)\n-        self.__class__ = GSObject\n+        return self.withScaledFlux(flux / self.getFlux())\n \n-    def setFlux(self, flux):\n-        \"\"\"Set the flux of the object.\n+    def withScaledFlux(self, flux_ratio):\n+        \"\"\"Create a version of the current object with the flux scaled by the given flux ratio.\n \n-        After this call, the caller's type will be a GSObject.\n-        This means that if the caller was a derived type that had extra methods beyond\n-        those defined in GSObject (e.g. getSigma() for a Gaussian), then these methods\n-        are no longer available.\n+        This function is equivalent to obj.withFlux(flux_ratio * obj.getFlux()).  However, this\n+        function is the more efficient one, since it doesn't actually require the call to\n+        obj.getFlux().  Indeed, withFlux() is implemented in terms of this one and getFlux().\n+\n+        It creates a new object that has the same profile as the original, but with the\n+        surface brightness at every location scaled by the given amount.\n+\n+        An equivalent, and usually simpler, way to effect this scaling is\n+\n+            obj = obj * flux_ratio\n+\n+        @param flux_ratio   The new flux for the object.\n \n-        @param flux The new flux for the object.\n+        @returns the object with the new flux.\n         \"\"\"\n+        new_obj = GSObject(self.SBProfile.scaleFlux(flux_ratio))\n+        if hasattr(self,'original'): \n+            new_obj.original = self.original\n+        else:\n+            new_obj.original = self\n+\n         if hasattr(self,'noise'):\n-            self.noise.scaleVariance( (flux / self.getFlux())**2 )\n-        self.SBProfile.setFlux(flux)\n-        self.__class__ = GSObject\n+            new_obj.noise = self.noise * flux_ratio**2\n+        return new_obj\n \n-    def applyExpansion(self, scale):\n-        \"\"\"Rescale the linear size of the profile by the given scale factor.\n+    def setFlux(self, flux):\n+        \"\"\"This is an obsolete method that is roughly equivalent to obj = obj.withFlux(flux)\"\"\"\n+        new_obj = self.withFlux(flux)\n+        self.SBProfile = new_obj.SBProfile\n+        if hasattr(self,'noise'): self.noise = new_obj.noise\n+        self.__class__ = new_obj.__class__\n+\n+    def scaleFlux(self, flux_ratio):\n+        \"\"\"This is an obsolete method that is roughly equivalent to obj = obj * flux_ratio\"\"\"\n+        new_obj = self * flux_ratio\n+        self.SBProfile = new_obj.SBProfile\n+        if hasattr(self,'noise'): self.noise = new_obj.noise\n+        self.__class__ = new_obj.__class__\n+\n+    def expand(self, scale):\n+        \"\"\"Expand the linear size of the profile by the given scale factor, while preserving\n+        surface brightness.\n+\n+        e.g. `half_light_radius` <-- `half_light_radius * scale`\n \n         This doesn't correspond to either of the normal operations one would typically want to\n-        do to a galaxy.  See the functions applyDilation and applyMagnification for the more\n-        typical usage.  But this function is conceptually simple.  It rescales the linear\n-        dimension of the profile, while preserving surface brightness.  As a result, the flux\n-        will necessarily change as well.\n+        do to a galaxy.  The functions dilate and magnify are the more typical usage.  But this","path":"galsim/base.py","position":372,"original_position":372,"commit_id":"74517aea2788539e8ad91e363db03090a169a68a","original_commit_id":"74517aea2788539e8ad91e363db03090a169a68a","user":{"login":"rmjarvis","id":623887,"avatar_url":"https://avatars.githubusercontent.com/u/623887?","gravatar_id":"4536b0aa642c3258af0c8117e627f17f","url":"https://api.github.com/users/rmjarvis","html_url":"https://github.com/rmjarvis","followers_url":"https://api.github.com/users/rmjarvis/followers","following_url":"https://api.github.com/users/rmjarvis/following{/other_user}","gists_url":"https://api.github.com/users/rmjarvis/gists{/gist_id}","starred_url":"https://api.github.com/users/rmjarvis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rmjarvis/subscriptions","organizations_url":"https://api.github.com/users/rmjarvis/orgs","repos_url":"https://api.github.com/users/rmjarvis/repos","events_url":"https://api.github.com/users/rmjarvis/events{/privacy}","received_events_url":"https://api.github.com/users/rmjarvis/received_events","type":"User","site_admin":false},"body":"Sure.  I'm fine with either convention.  In my doc regularizations on this branch, I didn't try to address this point: dilate vs `dilate` vs dilate().  I think I prefer the parens to indicate functions, but I suspect we often use the backticks in many places in the docs.  And I agree that either of these is better that just writing e.g. dilate without any marker.  \r\n\r\nIf we agree on the paren notation, I can try to go through and regularize this convention throughout.","created_at":"2014-04-11T12:36:59Z","updated_at":"2014-04-11T12:36:59Z","html_url":"https://github.com/GalSim-developers/GalSim/pull/514#discussion_r11529813","pull_request_url":"https://api.github.com/repos/GalSim-developers/GalSim/pulls/514","_links":{"self":{"href":"https://api.github.com/repos/GalSim-developers/GalSim/pulls/comments/11529813"},"html":{"href":"https://github.com/GalSim-developers/GalSim/pull/514#discussion_r11529813"},"pull_request":{"href":"https://api.github.com/repos/GalSim-developers/GalSim/pulls/514"}}}}