lsst-sims / legacy_sims_movingObjects

3 stars 5 forks source link

U/danielsf/debug #2

Closed danielsf closed 8 years ago

danielsf commented 8 years ago

The original implementation of __getitem__() did not allow for slicing of the orbits dataframe, which caused generateCoefficients.py to crash if you specified a limited range of objects. This pull request uses pandas' iloc (see http://pandas.pydata.org/pandas-docs/stable/indexing.html) to do the work in __getitem__(), which automatically enables slicing.

rhiannonlynne commented 8 years ago

Hi Scott - Can you add one more test (which I admit I should have done from the start) to check that if you try to "setOrbits" using a dataframe which does not start at 0 (i.e. you already sub-selected a dataframe) that this still works? (and in particular, that you can iterate through the new non-zero-starting orbit array?) I think I tried iloc first and found that I had issues .. but maybe some of the other indexing adjustments have actually fixed this.

rhiannonlynne commented 8 years ago

... although maybe the last test in testIterationAndIndexing actually does this already.

danielsf commented 8 years ago

Is this latest commit what you had in mind?

rhiannonlynne commented 8 years ago

Yes - although, sorry, I did realize after the fact that it was already included in the previous section of the unit test, really.

rhiannonlynne commented 8 years ago

Good to merge.