pyorbital icon indicating copy to clipboard operation
pyorbital copied to clipboard

add get all platforms function and generator getting one platform to processing

Open 19as opened this issue 4 years ago • 5 comments

  • [ ] Closes #xxxx
  • [ ] Tests added
  • [ ] Tests passed
  • [ ] Passes flake8 pyorbital
  • [ ] Fully documented

19as avatar Oct 15 '21 18:10 19as

please, merge my request in trunk

19as avatar Oct 30 '21 19:10 19as

Oh also could you try adding some tests for these functions?

djhoese avatar Oct 31 '21 00:10 djhoese

@19as Any updates on this? I had some questions and comments that I was hoping you could answer before we move forward with this pull request. If you have questions for me let me know. If you don't reply in the next week or two then I will plan on closing this.

djhoese avatar Dec 14 '21 18:12 djhoese

Sorry about the delay, we've been kind of busy and sometimes pull requests get lost in the inbox. How do you plan on using this functionality?

I know very little about TLE files so correct me if I'm missing something, but it looks like your two functions are very similar and possibly don't do what they say. The top one says that it gets multiple platforms and the bottom one says it gets a single one. However, the second one is a generator so it actually yields all the platforms too.

Lastly, these functions are almost exactly the same code. Could they be combined? Or one could use the other? If the only difference between these is a list versus a generator then I would suggest having only the generator and leave it up to the user to call list(get_platform(tle_file)) instead of doing it for them.

You're right. It is better to use one generator. I had to go through the file and, upon receiving one aircraft, process it

19as avatar Jun 16 '22 19:06 19as

Oh also could you try adding some tests for these functions?

I will try to do it in the future

19as avatar Jun 16 '22 19:06 19as