search

sagesharp / foss-heartbeat

(Unmaintained) FOSS Heartbeat analyses the health of a community of contributors. :heartbeat:
https://sarahsharp.github.io/foss-heartbeat/
Other
315 stars 38 forks source link

requirements.txt: Return statistics #36

Closed mureinik closed 7 years ago

mureinik commented 7 years ago

ghstats.py requires statistics in order to run. This requirement was originally featured in requirements.txt, but was removed by commit 3a79f8. That removal seems to be a mistake, as requiring statistics do not harm the functionality of ghrusthighfive.py that commit introduced, and it's clearly needed by ghstats.py.

This patch returns the statistics requirement in order to unbreak ghstats.py.

willingc commented 7 years ago

@mureinik Is there a reason that the statistics package would be used over the Python standard library statistics module? Or statsmodel or scipy? It seems as if there may be another issue with ghstats.py other than the import.

mureinik commented 7 years ago

@willingc to be completely frank, I don't have the foggiest. I cloned the repo and followed in instructions in README.txt to generate a report for a repo I was interested in (junit-team/junit5, although I think it's immaterial to the issue).

When I followed the instructions, running ghstats.py failed with my python 2.7.12 (and again, I admit I did not dive into its code):

mureinik@computer ~/src/git/foss-heartbeat [requirements] $ python ghstats.py junit5 junit-team docs/
Traceback (most recent call last):
  File "ghstats.py", line 41, in <module>
    import statistics
ImportError: No module named statistics

although it succeeded in my python 3.5.2.

Adding statistics to the requirements made ghstats.py work in python 2, and didn't seem to have any bad effects on the python 3 execution.

I wonder if separating the requirements.txt file to python3 and python2 files would be more correct, although this seems like a bit of an overhead (at least from my very superficial perspective).

willingc commented 7 years ago

Hi @mureinik,

Thanks for your response. I agree with you that adding this to the requirements file in the short term is a good solution without really any downside. I'll give ghstats.py a look to see if there is something in there that needs to be updated. If so, I'll open another PR. Thanks!

@sarahsharp I would recommend merging this for the short term so that folks can be productive.

mureinik commented 7 years ago

@willingc check out the updated revision of this PR I just pushed. It limits requiring statistics to older python versions (pre-3.4), which would prevent it from installing redundant modules on new python environments and make it a tad more palatable.

mureinik commented 7 years ago

@sarahsharp @willingc This PR has been sitting here for a while, and I'd like to get it off my radar. If you think this attempt is misguided, I'll just close it (although I'd appreciate an explanation as to why). If it still needs some work I'd love to do it, but I'll need some guidance as to what this PR is lacking. Otherwise, if you think it's a worthwhile contribution, can it be merged?

willingc commented 7 years ago

Hi @mureinik, Sorry for the delay in getting this merged. The PR looks good to me. Thanks for your patience and your contribution. :cake: