Repository navigation
Add a double-precision Phase class - #77
Conversation
10259e2 to
f6a5d92
Compare
f6a5d92 to
d5794d4
Compare
98c0d62 to
d9fd67b
Compare
37939cb to
18217d0
Compare
|
@luojing1211 - there are things that still could be improved, but for our purposes the |
This particular branch is a mix of ideas from EarthLocation and Time.
… double. Aslo add floor division, modulo, and divmod.
Also add test for isnan.
This as we need __array_ufunc__, and sufficient Quantity support. Also update Phase to have its own max, min, ptp, and sort methods. Since those of Time refer to jd1, jd2, etc., and have links that will fail.
Allow initialization with Phase, and anything that would initialize an Angle.
Plus some smaller changes.
Since older astropy may not support that version.
Add support for rint.
This if for now. Some reductions, such as np.add, np.minimum, np.maximum, would be not that hard to support.
18217d0 to
c73e2f0
Compare
|
@luojing1211 - would you have time to have a look at this PR? If not, no problem, then I think I'll just merge it - it should be reasonably fine. |
|
I don’t have a issue with it. But I had some comments on the PR.
On Tue, Jun 4, 2019 at 1:29 PM Marten van Kerkwijk ***@***.***> wrote:
@luojing1211 <https://github.com/luojing1211> - would you have time to
have a look at this PR? If not, no problem, then I think I'll just merge it
- it should be reasonably fine.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#77>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABIUBA3TBFYEDQBAMS2Q64TPY2Q6VANCNFSM4HCSAJMQ>
.
--
-Jing
|
|
@luojing1211 - great, let me know if you'd want to put them in-line or rather sit together and adjust. |
| @@ -199,16 +199,19 @@ def _get_offsets(self, samples, precision=1.e-3, max_iter=10): | |||
| return (np.round(samples / self._mean_offset_size + | |||
There was a problem hiding this comment.
Is this function aiming to solve the non-uniform phase stack problem?
There was a problem hiding this comment.
This branch is for the case that one is just integrating by some amount of time.
| np.equal, np.not_equal, | ||
| np.less, np.less_equal, np.greater, np.greater_equal} | ||
|
|
||
|
|
There was a problem hiding this comment.
It is useful to have a small function that converts a string number to two doubles. I am not sure if this functionality should be added to this PR.
There was a problem hiding this comment.
Do you mean a string `fffffff.fffffffffffffffff'? Ie., decoding it like for DADA times?
It would not be difficult to do. Would a classmethod be OK, or do you think the initializer itself should deal with it? (Slightly prefer the former myself).
There was a problem hiding this comment.
Yes, decoding it like DADA times, or a big number of phase. From the point of the user, I think the initializer one would be handy.
There was a problem hiding this comment.
I like the idea, but think we should then also allow writing such long strings. So, I made it a separate issue: gh-93
|
Let me merge this one, so we have a basis to work of for future improvements. |
With the idea to move this up to PINT, to replace its phase class.
fixes #71