Skip to content

Conversation

@BatchelorJ
Copy link
Contributor

This PR standardizes the naming of transformation definitions in the constants module (resolves #129) and adds reverse transformations utilizing the same naming convention (resolves #127). Several other modules and tests that use some of these transformation objects have been refactored, and the names of functions that convert GDA2020 <-> ATRF2014 have been changed so that they have different names to the transformation objects themselves.

…ing of variables in other modules and tests

Signed-off-by: SGV-Geodesy <56060078+SGV-Geodesy@users.noreply.github.com>
@BatchelorJ BatchelorJ requested a review from harry093 May 19, 2021 05:28
mga94_to_mga2020,
mga2020_to_mga94)
from geodepy.constants import itrf14togda20, gda94_to_gda2020
from geodepy.constants import itrf2014_to_gda2020, gda1994_to_gda2020
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it best use stick with common usage. So, GDA94 but GDA2020. Similarly, the ITRF versions pre-2000 have only two digit years (ITRF92), while those from 2000 on have four digits (ITRF2014).



def atrf2014_to_gda2020(x, y, z, epoch_from, vcv=None):
def conform_atrf2014_to_gda2020(x, y, z, epoch_from, vcv=None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

transform_atrf2014_to_gda2020 may be a clearer function name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@harry093 good idea, I've also extended this to add transform_ prefix to all datum-to-datum functions in the transform module for clarity

Copy link
Collaborator

@harry093 harry093 left a comment

Choose a reason for hiding this comment

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

@BatchelorJ thanks for this. I am open to being convinced otherwise, but I think we should follow the standard names - as frustrating as they are. See my comment in test_transform.py

@BatchelorJ
Copy link
Contributor Author

@BatchelorJ thanks for this. I am open to being convinced otherwise, but I think we should follow the standard names - as frustrating as they are. See my comment in test_transform.py

Thanks @harry093, I thought I'd throw this code up to see what we thought about it so thanks for the feedback. I'm happy to stick to the rule of years <2000 using 2-digit suffixes while >=2000 using 4-digit suffixes as long as it's consistently applied. Most in the geodetic and survey community understand this convention so we should be able to get away with it.

…datum-specific transformation functions with transform_ prefix

Signed-off-by: SGV-Geodesy <56060078+SGV-Geodesy@users.noreply.github.com>
@BatchelorJ BatchelorJ requested a review from harry093 May 21, 2021 04:07
sc=-1.854,
rx=0.008, ry=-0.557, rz=-0.178)

gda19_to_itrf2008 = -itrf2008_to_gda94
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be gda94_to_itrf2008? Same typo made below as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@harry093 well spotted! All fixed up in latest commit

Copy link
Collaborator

@harry093 harry093 left a comment

Choose a reason for hiding this comment

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

@BatchelorJ looks great mate. I think there's a small typo in the constants module and once that's fixed we are good to merge

Signed-off-by: SGV-Geodesy <56060078+SGV-Geodesy@users.noreply.github.com>
Copy link
Collaborator

@harry093 harry093 left a comment

Choose a reason for hiding this comment

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

@BatchelorJ all good! Thanks

@BatchelorJ BatchelorJ merged commit 2392b94 into master May 24, 2021
@BatchelorJ BatchelorJ deleted the tranformation_standardisation branch May 24, 2021 22:54
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.

Standardise names of Transformation objects, etc Define reverse transformations

4 participants