Unity-Technologies / pysolotools

Python toolchain for SOLO.
https://Unity-Technologies.github.io/pysolotools
Other
39 stars 14 forks source link

[chore]: Adding some test coverage for obvious things #106

Closed fsudrew07 closed 2 years ago

fsudrew07 commented 2 years ago
  1. Added testing on Windows machines to the GitHub action unit tests
  2. Added coverage for the StatsHandler

@Saurav-D and @86sanj - Can I propose removing "Cat_IDs" as a parameter to the handler? Not all analyzers will take that value as an input and to me it would make sense to just add that to the constructor of the individual Analyzer classes. What do you think? I didn't want to just go and do that right now without checking with you both first as I think you're working on adding classes and such.

Secondly - There's a JsonSerializer and JsonDeserializer class - what is the purpose of those? They aren't used anywhere, can they be deleted as opposed to writing tests and maintaining code that isn't used?

github-actions[bot] commented 2 years ago

Coverage

Coverage Report
FileStmtsMissCoverMissing
pysolotools\clients\gcs_client.py29872%12, 33–38, 40
pysolotools\consumers\solo.py39685%53–54, 113, 119, 139, 145
pysolotools\converters\solo2coco.py186597%60, 125, 270, 311, 386
pysolotools\core\iterators\frame_iterator.py41198%95
pysolotools\core\models\solo.py2361693%138, 148–150, 181, 189, 280–285, 288, 333, 336, 362, 365
pysolotools\stats\analyzers\base.py9278%24, 40
pysolotools\stats\analyzers\bbox_analyzer.py42420%1–109
pysolotools\stats\deserializers\base.py660%1–21
pysolotools\stats\deserializers\json_deserializer.py770%1–20
pysolotools\stats\serializers\base.py9367%11–12, 23
pysolotools\stats\serializers\json_serializer.py22220%1–41
TOTAL68611883%  

Tests Skipped Failures Errors Time
41 0 :zzz: 0 :x: 0 :fire: 5.926s :stopwatch:
fsudrew07 commented 2 years ago

I agree with removing cat_ids and moving it to the constructor instead. As for the JSON serializer, I guess that's for when we want to write all the results to a JSON file. @86sanj can give more information about that.

Thanks for taking a look. re: the JSON serializer - I think my biggest quibble against it is that it's also assuming that I want to not only write the data to a file, but write the data to a file locally. Let me take a stab at separating some of the concerns here.