Skip to content
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

_latlon: Fix conversion from DMS to decimal degrees #3

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

VOMAY
Copy link

@VOMAY VOMAY commented Sep 26, 2018

No description provided.

@VOMAY
Copy link
Author

VOMAY commented Aug 23, 2019

I'd love to see this fix merged. Best Regards

@ccrook
Copy link
Contributor

ccrook commented Aug 24, 2019

Hi @VOMAY Sorry I missed this - got lost in the noise :-( I definitely want to add the bug fix in the request in the use of string replace - thanks very much for identifying it. I'm not sure about the wrapping at 180 though - I'm not sure that this is a function of reading the SINEX file. More a decision about what to do once it is read. Particularly coming from NZ, near 180, I'd need to check the impact of this on downstream software. So at the moment my preference would be to not include this without further reading.

@VOMAY
Copy link
Author

VOMAY commented Jan 10, 2020

Dear @ccrook I understand your concern with the 180 in NZ, although I think keeping the range between -180 and 180 is the result expected by most users. I cannot recall reading longitude > 180 is a SINEX file.

@ccrook
Copy link
Contributor

ccrook commented Jan 10, 2020

@VOMAY I have added your fix for negative angles (hence the merge conflict) - thanks again for that. I'm still not inclined to modify the range of longitudes. My reason is that if that is what is in the SINEX file then that is what I want to see. If your SINEX files don't have longitudes greater than 180 then I guess this code wouldn't be used in any case. I'm sorry this isn't what you want but that is my preference at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants