riogroup / SORA

MIT License
15 stars 12 forks source link

New Chord and ChordList classes #53

Closed altairgomes closed 3 years ago

altairgomes commented 3 years ago

New features are being introduced. These are new classes Chord and ChordList which will intermediate between the Occultation class and the Observer and LightCurve classes. These new classes are not new modules, but they are part of the occultation module. A new List class was also introduced as a Base Class for other list classes that may be created for SORA, for instance, RingList.

altairgomes commented 3 years ago
  1. If we instantiate the Occultation using Ephem instead of Body, a error occurred;

What error? Does Ephem have a name to search in the SBDB? If it is a satellite, it should be instantiated with Body. If there is an error, it's not for this PR.

  1. There is a bug in the Occultation.get_map_sites(), all the stations are colored as red;

I'll look into it. They should all be red if no chord is positive.

  1. If we add a chord using Occultation.Chords.add_chord(), using the object chord as input there is a error;

How do you define the chord? If it is a chord that's already added, it won't add it again.

  1. Occultation.plot_chords(all_chords=False) is not ignoring the disabled chords.

I'll look into it.

  1. It seems that del(Chord) do not eliminate the name from the list;

if you mean

c = occ.chords.add_chord(...)
del(c)

That's is how python works. c is not the object, is a pointer to the object. If you want to remove from the list, you should use occ.chords.remove_chord(name="name_of_chord") Even if you do that, cwill still point for the object, that is not attached to any Occultation.

  1. I think Chord.get_fg() should have a warning considering the imputed time, similar to Occultation.new_astrometrical_position();

Ok.

  1. Occultation.Chords.plot_chords() should have a kwarg similar to All_chords. Instead of only the kwarg ignore;

Ok.

  1. I'm not sure if the Chord.path() is useful to the user, maybe it should be a hidden function.

I think hidden function are those we do not want for the user to run. Maybe because they may generate an error, or whose calls should be limited. This is not the case for the path function. If a user wants to use, it is there available.

Bmorgado19 commented 3 years ago

Approved, please update the Releases.rst