Closed jtrutna closed 6 years ago
Can't comment on the line because it's not in the diff but "altitude" is in the README.md twice with two different descriptions (in the quick_find API section)
Didn't see any bugs. Daylight clock has runout. If you don't merge it I'll dig deeper tomorrow. Not sure how rigorous people are being with their lgtm's.
Quite thorough. It should probably take a couple hours to review a PR. You need to go through every line, understand it, and be quite sure it's correct. You can push back to the submitter if the code isn't clear enough to do that, of course.
You can safely assume every submission has a bug and it's your job to find it.
gsoap/gsoap.py
: (branch: gcwip2
)
Throws a cpredict.PredictionException
when called with no arguments. Can't seem to catch it with normal try/except. Ran out of time to debug. Not sure if it's related to this code.
import gsoap
# <generator object <genexpr> at 0x114a280>
next(gsoap.transits())
#Traceback (most recent call last):
# File "<stdin>", line 1, in <module>
# File "gsoap.py", line 46, in <genexpr>
# return (transit for _, transit in heapq.merge(*transit_generators))
# File "/usr/lib/python2.7/heapq.py", line 341, in merge
# h_append([next(), itnum, next])
# File "gsoap.py", line 44, in <genexpr>
# transits = ((t.start, t) for t in predict.Observer(sat.tle, gs.qth()).passes())
# File "/usr/local/lib/python2.7/dist-packages/predict.py", line 64, in passes
# transit = quick_predict(self.tle, ts, self.qth)
#cpredict.PredictException: Satellite has decayed. Cannot calculate transit.
def transits(satellites=None, stations=None):
if satellites is None:
satellites = theknowledge.satellites.list()
if stations is None:
stations = theknowledge.groundstations.list()
transit_generators = []
for sat in satellites:
for gs in stations:
transits = ((t.start, t) for t in predict.Observer(sat.tle, gs.qth()).passes())
transit_generators.append(transits)
return (transit for _, transit in heapq.merge(*transit_generators))
Oh right. Gotta mention the other person. @jtrutna
Other then described - lgtm
Way old and overcome by events.
@ttrutna Big reorganization resulting from #3 and use cases.