search

cheind / py-motmetrics

:bar_chart: Benchmark multiple object trackers (MOT) in Python
MIT License
1.37k stars 259 forks source link

add `if metrics is None` check to compute_many #107

Closed cinabars closed 4 years ago

cinabars commented 4 years ago

when calling compute_many, if kwarg metrics is left to its default value None, it will not calculate all metrics. This does not align with the Kwargs documentation on this function, nor the behavior of similar function compute.

jvlmdr commented 4 years ago

Thanks for this! We should definitely fix this to standardize the behavior across the different compute functions and to match the docstring.

I just noticed that we use motchallenge_metrics as the default in MetricsHost.compute() and self.names as the default in MetricsHost.compute_overall() (and I can't see where this is set).

Should we also replace self.names with motchallenge_metrics at the same time? @cheind

jvlmdr commented 4 years ago

Also strange that appveyor failed with "Build execution time has reached the maximum allowed time for your plan (60 minutes)." It seems highly unlikely that this is due to the content of this PR.

cheind commented 4 years ago

Sorry for the late response, I'm on vacation. Yes I believe that is an inconsistency. self.names seems to be are oversight from a previous refactoring and should be removed in factor of motchallenge_metrics.

cheind commented 4 years ago

We've seen these random AppVeyor fails before. As far as I can tell it seems to be related to uploading to pypi-test. We might be lucky and the error goes away without changes in a future attempt. Trying to restart the build, now.

Seems not to have helped. I will update twine upload tomorrow. Keeping this https://packaging.python.org/guides/using-testpypi/ as a reference.

cheind commented 4 years ago

The Appveyor issue is now fixed (df8dbfa) on develop. @cinabars would you mind rebasing your PR on the latest changes?

For the record: the way we handled version numbers worked only for master and development branches, but not for PR and feature branches. Uploading failed because of conflicts with already existing non-development package names (i.e no .dev### suffix) on PyPi.

cinabars commented 4 years ago

Pulled down the fix, looks like the appveyor build is still failing though.

cheind commented 4 years ago

Sorry for the inconvenience. I've disabled pypi uploading for PRs for now. Would you mind rebasing on the latest develop? 2436d4f

cheind commented 4 years ago

Hah, that will also not work as Appveyor marks pull requests as develop. APPVEYOR_REPO_BRANCH - build branch. For Pull Request commits it is base branch PR is merging int

cheind commented 4 years ago

So, according to docs it seems like before_deploy scripts are preferrable. Those are never run for PRs. If you find the time, please rebase one more time :) If that doesn't help we will probably accept your PR anyways as all the tests pass.

cheind commented 4 years ago

It seems to work now. The main issue seems to be this: building a PR does not decrypt sensitive variables, causing PyPi to fail to access our account.

cheind commented 4 years ago

Thanks and sorry for the problems with our continous integration services.