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

Change some long method signatures to require keyword arguments? #1223

Closed
mkolopanis opened this issue Nov 10, 2022 · 7 comments · Fixed by #1333
Closed

Change some long method signatures to require keyword arguments? #1223

mkolopanis opened this issue Nov 10, 2022 · 7 comments · Fixed by #1333

Comments

@mkolopanis
Copy link
Member

python supports the * delineation in a method signature to require everything after to be keyword only (and / to signify everything before be positional only). Should we change some of our longer methods (e.g. read) to have the keyword only arguments?

https://peps.python.org/pep-3102/

@steven-murray
Copy link
Contributor

absolutely yes please

@bhazelton
Copy link
Member

Ok, we'll do this in version 3.0. We don't currently know how to do a deprecation process for this, so doing it in a major version change is most appropriate.

@bhazelton bhazelton added this to the Version 3.0 milestone Dec 15, 2022
@bhazelton
Copy link
Member

Looking at this a bit and I have some questions. How aggressive do we want to be on this? Is this mostly for developers so we don't have to worry about changing parameter order, or is this mostly to protect users from accidentally passing parameters in the wrong order?

An example that I'm thinking about is the utility function XYZ_from_LatLonAlt. It can be confusing whether lat or lon comes first (although the function name should make this clear), so I could see requiring all the inputs to be passed by name. But there are only 3 inputs and I think this is pretty widely used, so this change might affect a lot of users. Thoughts?

@steven-murray
Copy link
Contributor

IMHO that particular example should not be changed since the order of arguments is in the function name itself.

@bhazelton
Copy link
Member

IMHO that particular example should not be changed since the order of arguments is in the function name itself.

Thanks. What about ENU_from_ECEF, which has this signature: ENU_from_ECEF(xyz, latitude, longitude, altitude, frame="ITRS"). Should I make lat, lon, alt be passed by name?

@steven-murray
Copy link
Contributor

I think so, yes. Because those are non-obvious.

@bhazelton
Copy link
Member

fixed in #1333

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