Skip to content

DM-7162: adding first skeleton and test for lightcurve handler #161

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

Merged
merged 3 commits into from
Sep 9, 2016

Conversation

ejoliet
Copy link
Contributor

@ejoliet ejoliet commented Aug 31, 2016

I've added two processors (maybe it can be combined). They are both under firefly/server/query.

One is taking care of returning periodogram and peaks tables. The other is in charge of phase folding an input IPAC table (raw LC table).

I've added a handler to deal with the Nexsci API in the future. Ans an IRSA handler to deal with the extraction/parsing of both tables from the API result VOTable xml.
There is also a test to demonstrate how to get the tables out of a request.

For now, i'm faking the return API with a sample VOTable.

The request object (PeriodogramAPIRequest) expects input parameters needed for computing the periodogram (calling the API) and phase folding a raw light curve. (Note: Maybe we need to add the URL as part of the request so we can always change the API url directly from the client instead of doing it from the server?)

The phase folding is under development and need to be integrated in the handler. For now i'm returning the same raw table that i've passed in as input in the request.

I've added the test buttons in firefly-dev.html, they appear in the tab 'Compute periodogram' in the TestQueriesPanel (.jsx) component.

More offline discussion might be needed though.

Please, let me know if that make sense regarding the feature requirements and if it covers the future need.

@@ -0,0 +1,29 @@
package edu.caltech.ipac.firefly.server.query;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please include on top of file the caltech/ipac license's information. See other source file for example.

@loitly
Copy link
Contributor

loitly commented Sep 1, 2016

Beside the placement of the added classes, I would also like to see how they can be simplified.
The multiple level of abstractions may not be necessary. Can we talk about it?

@ejoliet
Copy link
Contributor Author

ejoliet commented Sep 8, 2016

Cleanup and move to .lc package. Please, have a look.

@loitly
Copy link
Contributor

loitly commented Sep 9, 2016

It looks good. Review is complete.

@ejoliet ejoliet merged commit da91926 into dev Sep 9, 2016
@ejoliet ejoliet deleted the DM-7162-add-lc-processor branch September 9, 2016 21:47
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.

2 participants