search

sigmf / sigmf-python

Easily interact with Signal Metadata Format (SigMF) recordings.
https://sigmf.org
GNU Lesser General Public License v3.0
44 stars 17 forks source link

Fixes #51 #59

Closed gregparkes closed 5 months ago

gregparkes commented 5 months ago

This PR:

Undergone basic testing using Pytest and linting via black.

777arc commented 5 months ago

Awesome set of fixes!

Teque5 commented 5 months ago

I made a lot of mods and added some test code, but this is working how I expect now and it looks okay to merge. I'll give you a day to respond before I merge if you want to make any other changes @gregparkes.

gregparkes commented 5 months ago

OK @Teque5 thanks for the changes - the 'multiple globstar' is a nice touch using the + command on argparse.

There are a number of inefficiencies in terms of speed for certain edge cases:

  1. If only 1 file is selected (after all the globs), there's some overhead introduced in still producing the ThreadPoolExecutor for one task and computing asynchronously.
  2. The check for no available files comes after the ThreadPoolExecutor, which again could lead to overhead.

But these are relatively small beans I'll admit.

I'm adding some slight tweaks in terms of type hints, an unused import (Path can be removed now), and mypy spotted a naming error (using for error in validate_properties where error is also an imported variable).

Commit incoming.

Teque5 commented 5 months ago

Thanks for the PR @gregparkes!