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

Confusion over name, id, frame properties #177

Open
scsides opened this issue May 31, 2019 · 4 comments
Open

Confusion over name, id, frame properties #177

scsides opened this issue May 31, 2019 · 4 comments
Milestone

Comments

@scsides
Copy link
Contributor

scsides commented May 31, 2019

There is confusion over what some of the properties are when they are returning a sting name, naif code, naif frame code. We should make this clear and consistent asap.

Possible Proposal:
xxxxx_name = returns a long name
xxxxx_short_name = returns an abbreviated short name
xxxxx_id = returns an integer
xxxxx_frame_code = returns a naif from code

@jessemapel
Copy link
Contributor

In what context are xxxxx_id and xxxxx_frame_code different for a reference frame? I think we can go with just xxxxx_id.

@scsides
Copy link
Contributor Author

scsides commented May 31, 2019

As far as I know they are the same, just two different things we could standardize on

@SgStapleton
Copy link
Contributor

SgStapleton commented May 31, 2019

For example, in NaifSpice, the instrument code (btw - why are we calling it ikid instead of something more descriptive like instrument_code):

    @property
    def ikid(self):
        return spice.bods2c(self.instrument_id)

We are converting a STRING to a CODE here (given by the Spice call), yet the string is referred to as "instrument_id", but "id" is in the property name returning an int. Are ids strings or ints??? Or should we be following the standard already laid out for us in Spice and call ints codes (or idcodes or...)?

Also, is the Naif code for a body, for example, not different than the code for the body frame? That is actually a question I had for this frame chain implementation. Again, using NaifSpice as an example:

@property
def target_id(self):
    return spice.bods2c(self.target_name)

@property
def target_frame_id(self):
    return spice.gipool('BODY_FRAME_CODE', 0, 1)

Seems like we are going about getting these values differently, so are they not different? (I just realized that I implemented target_frame_id in my last PR, so it looks like I went about this all wrong anyway, so maybe ids and frame ids are the same after-all?)

Iit seems like we are having inconsistent naming or naming that just does not seem to convey what is actually being accomplished, as well. Again in NaifSpice:

    @property
    def reference_frame(self):
        return 'IAU_{}'.format(self.target_name)

Reference frame what? Name, code, class, etc. ? Also, what is it a frame for? Spacecraft, sensor, body...? You can see that this is for the target body by reading the logic, but should we not name our properties so that this is obvious without having to check the logic?

@SgStapleton
Copy link
Contributor

I do like the idea that @scsides proposed about conventions above.

@jessemapel jessemapel added this to the 0.2.0 Release milestone Jul 2, 2019
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

No branches or pull requests

3 participants