-
Notifications
You must be signed in to change notification settings - Fork 88
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
Major upgrade of TaQL #388
Conversation
|
Thanks for assigning it. I have tested this branch previously offline. I think we need more feedback before merging this large commit. Could someone run the Casa test suite of this branch? Any thoughts @juliantaylor? |
the casa testsuite fails with this change: two parse errors:
and one other failure:
|
The parse error is a nice catch. Current trunk works:
Whereas the branch fails:
|
Setting to 'dontmerge' to prevent accidentally merging this. At least we should look into the casa test failures. |
I have a build of this branch running in a web interface (to be announced when it's more finished). But it might be useful for having a look at some of the new features. http://taql.astron.nl |
I had to remove the possibility to enclose a set in parentheses, because it I do not understand the 2nd problem. It seems like a coordinate problem, On Mon, Apr 4, 2016 at 2:44 PM, Julian Taylor notifications@github.com
|
I've looked at the parse error and I must confess I was a bit sloppy. On Mon, Apr 4, 2016 at 3:19 PM, Ger van Diepen gervandiepen@gmail.com
|
@juliantaylor , could you paste the exception-throwing lines of gcwrap/tools/images/image_cmpt.cc (around line 2241, in the version on your jenkins) here? Perhaps that will give Ger a clue on what the offending change was (and possibly how it can be fixed, in either casa or here). |
actually the line I pasted was not the test failure ... it was another test checking a mean which exceeded the tolerance. But that test uses random numbers and does not fail anymore, maybe it was just a random deviation. |
I mean the changes to |
In e6d346a, @gervandiepen changed the input to another date, so that it does not depend on measures data newer than 2016. With changing the input date, also the output of the measures conversion changes. |
The input date is in |
ok good, the failure was probably just a fluke then. So only the set parenthesis issue remains. |
@gervandiepen It would be great if you could keep the |
I can keep it and I've already made the change in my working space. On Tue, Apr 5, 2016 at 4:04 PM, Tammo Jan Dijkema notifications@github.com
|
@juliantaylor said that was probably just a fluke. Not sure what that is, but it seems to have gone away. |
Checks pass now. Unless someone objects, I'll merge this tomorrow. |
TaQL has been extended with several new features: