GliderGeek / PySoar

Gliding competition analysis
http://www.pysoar.com
GNU General Public License v3.0
17 stars 7 forks source link

Fix typo in excel export preventing analysis to be exportet #172

Closed condoratberlin closed 9 months ago

condoratberlin commented 10 months ago

a typo prevents creating a report. This is fixed with this pull request

GliderGeek commented 10 months ago

Hi @condoratberlin , thanks for your contribution. Can you explain a bit more what went wrong before this fix? Thanks!

condoratberlin commented 10 months ago

Sure, simply starting the app on windows or from en ide allows to pass a link from a daily result. After downloading and analysis a key error ('text_bst' not found) was raised and the excel analysis was not generated and could not be shown. After fixing 'text_bst' to 'text_best' and 'text_wrst' to 'text_worst' the same process finished without an error and the analysis is generated and could be opened.

GliderGeek commented 10 months ago

I have tried with a URL but are not able to replicate it. Can you share a competitionday triggering the error? I think this is a latent bug btw, but maybe it's not triggered because all the text entries in the settings are "neutral"?

anyway, sorry for this horrible code, this has been written a long time ago and is the project on which i started programming 🙈

condoratberlin commented 10 months ago

Ok, I try to create a small test tomorrow, wich replicates the behaviour.

condoratberlin commented 10 months ago

@GliderGeek I added a pytest, which raise an error before my fix due to a keyerror in the AnalysisThread and passes after my fix. Maybe it is a first entrypoint for automated tests? It was difficult to assert the errors in the separate thread. I#m pretty sure there are more elegant solutions I didn't know.

GliderGeek commented 10 months ago

Thanks for adding the URL. I'm not sure i would like to add end to end tests over the complete application since I haven't seen many bugs in this layer. opensoar provides most of the functionality and that is properly tested. the exportclass in an exception to this, but im not convinced it's worth the effort at the moment to add proper testing.

Looking into the bug i find that something else is off when i "fix" the problem. It tries to rank the "Airplane" column which should not be ranked. So i believe something else is going wrong, still need to figure out what though. The bst and wrst typos should never be used I think (text is never ranked)

image
GliderGeek commented 10 months ago

The problem seems to be the empty competition id on the original soaringspot site

image

strangely enough when you inspect the site source it is possible to pull out the IGC download link

this is not trivial to fix correctly since a few places in opensoar/pysoar assume that there is a competition id

GliderGeek commented 10 months ago

@condoratberlin i found the source of the problem and have addressed this in a new version op opensoar: v1.1.2.

maybe we can use this pull request to update pysoar to this version?

it's nice to still fix the typo, but can you remove the automated testing?

condoratberlin commented 10 months ago

Perfect. I remove the test tomorrow. Thanks for digging into the real problem.

condoratberlin commented 10 months ago

@GliderGeek Tests are removed and opensoar is updated to 1.1.2

GliderGeek commented 9 months ago

@GliderGeek Tests are removed and opensoar is updated to 1.1.2

nice, just appended an updated changelog such that i can create a release.

GliderGeek commented 9 months ago

@condoratberlin : the new version is released, thanks for your contribution :)