Skip to content

IRSA-327: fix rounding problem #345

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 1 commit into from
Apr 10, 2017

Conversation

ejoliet
Copy link
Contributor

@ejoliet ejoliet commented Apr 7, 2017

I've set a default min precision for VOTable converter to data group, plus added the reading of a precision attribute from VOTable if it exists.
The guessing part of the format is only made in the first row. We need to improve that also for the IPAC table reader which is related to ticket raised during GOODS data-set release (IRSA-128).

Please check that this make sense and doesn't affect other things.

To test: do a periodogram calculation with period min=1, period max=2, rest as default for example, the first value of the VOTAble from the API is 1, the second is 1.000...
This value should appear in the table/xy plot view with at least 8 digit which is the default minimum precision.

… VOTable, reading precision VOTable if it exists
@ejoliet ejoliet requested review from wmiipac, lznakano and loitly April 7, 2017 22:10
Copy link
Contributor

@wmiipac wmiipac left a comment

Choose a reason for hiding this comment

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

math looks right. tested with periodogram calculation, the precision shows the minimum of 8.
Question: is the precision minimum(8) set by the VOTable standard?

@ejoliet
Copy link
Contributor Author

ejoliet commented Apr 7, 2017

Precision is not yet generated from back-end service, Walter needs to add it. But the code is prepare to handle it ;-)

Copy link
Contributor

@loitly loitly left a comment

Choose a reason for hiding this comment

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

The change looks fine. Good to merge.

@ejoliet ejoliet merged commit 5e647fb into rc Apr 10, 2017
@ejoliet ejoliet deleted the IRSA-327-periogoram-period-increase-precision branch April 10, 2017 20:41
@xiuqin
Copy link
Contributor

xiuqin commented Apr 11, 2017

@ejoliet if the precision is not set, the default is 8. Correct?

@ejoliet
Copy link
Contributor Author

ejoliet commented Apr 11, 2017

Correct, but we can always change the default precision later. This is only applied to VOTable reader. We still have the problem for IPAC tables...

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.

4 participants