ligerbots / dslogparser

Parse FIRST FRC driver station log files
MIT License
16 stars 10 forks source link

Any interest in packaging this up? #3

Closed jaustinpage closed 4 years ago

jaustinpage commented 4 years ago

Hi @prensing ,

I was curious if there is any interest in packaging this up as a library for reuse? For instance, I am planning on using https://github.com/ligerbots/dslog2csv/blob/master/dslog2csv.py#L202 in a project that will do some log analysis.

Thank you so much for putting this together, I really appreciate it!

jaustinpage commented 4 years ago

After reviewing the forks of this project, it seems like there is some precedence for others wanting to reuse the core parser. https://github.com/relishcolouredhat/dslog2csv/blob/master/dslog2ifql.py is a good example of wanting the same core parser, but, output in a different format.

jaustinpage commented 4 years ago

I have created a new repo, https://github.com/jaustinpage/dslogparser, which shows a minimal example of how to turn this into a library.

if you want to test this before setting up pypi, you can do a pip install git+https://github.com/jaustinpage/dslogparser, then follow the rest of the readme https://github.com/jaustinpage/dslogparser#dslogparser to test it.

I would be happy to transfer the library to the ligerbots organization if you would like.

it could also be published to pypi, see https://python-packaging.readthedocs.io/en/latest/minimal.html#publishing-on-pypi

prensing commented 4 years ago

Packaging sounds like a fine idea. I will take a look, but it will probably be a few weeks before I have time.

jaustinpage commented 4 years ago

You could, if you wanted, make a new repo on ligerbots called dslogparser. I'll make a pr to it that makes it a package. Once that is merged in, I'll make pr to this repo, refactoring it to use the library. This would put the burden of doing the work on me, while allowing you to continue to control the project

prensing commented 4 years ago

We can talk via email, but just to make it more public: I sucked in most of your changes. I think there are only 2 serious differences:

jaustinpage commented 4 years ago

We can talk via email, but just to make it more public: I sucked in most of your changes. I think there are only 2 serious differences:

  • I made find_match_info() a staticmethod of DSEventParser. It takes a filename. I kind of hate file seeks.
  • find_match_info() returns a dictionary, like the other routines, for consistency.

Yep, these changes look good to me. The tradeoffs for code simplicity are probably good ones in this case. I am also good with marking this particular issue as "solved"