-
Notifications
You must be signed in to change notification settings - Fork 3
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
Enhanced light curve plotting behavior #102
Comments
I just remembered another issue that crops up when plotting multifrequency signals. Since a signal with 2 or more incommensurable periods is not truly periodic, when we phase it, the interpolating function is ambiguous. From times
Perhaps we should just support both methods, and use a different option for each? Either way we go, it will require a decent bit of refactoring, so we should think before we code. There's also the question of which time interval to use for option 1, which is not a very easy question. Perhaps we do something like always use |
The more I think about it, the more it seems like we just need to support both options 1. and 2. The remaining questions then are how we choose the time interval for 1., and how we separate these options. Probably with two output specifiers, like |
Actually, there is a 3rd and 4th possibility for plotting, which I just remembered. 3 . Same as 2., but take the average at each phase point.
4 . Compute the interpolating function using only the coefficients which correspond only to the one period used.
I honestly think all of these plot types are desirable in some sense, but it should also be possible to produce them externally, using the tabular output, so we don't necessarily need to implement them all within plotypus. Maybe we should just implement option 2., and leave out the rest. Another advantage of this plot type is that in the singly-periodic case, it reduces to the existing behavior. |
Currently,
plotypus.lightcurve.plot_lightcurve
expects phased data. It takes a period as well, but only for display on the time axis. To better support multi-frequency fits, I propose the following changes.Most importantly, the function should expect unphased data, as the meaning of phased becomes less clear-cut in this case.
If
period=None
, then the unphased data should be plotted as-is, with the fit plotted frommin(time)
tomax(time)
. This solves part of what was proposed in #40.If
period=scalar
, then phase the data with that period. This will produce the same output as it currently does, the only difference being the input (phased/unphased).If
period=array-like
, return a list with the same dimensions, and each element of the list containing the result of recursively callingplot_lightcurve
withperiod=element in array-like
. This can actually easily support arbitrary-dimensional arrays, through the use of recursion.In the command line program, we should separate the option
--output-plot-lightcurve
intophased
andunphased
versions (still deciding on the naming). This way it will only callplot_lightcurve
withperiod=None
if the unphased version is requested, and withperiod=result["period"]
if the phased version is.One thing that doesn't quite mesh with this approach is the current
output
argument toplot_lightcurve
. We would need some sort of way to get indexing information to it. Perhaps it would be best to simply move saving from the plot function itself tocli.py
, since it has access to all of thematplotlib.Figure
objects.The text was updated successfully, but these errors were encountered: