Add python version to runtime info #50
Conversation
Codecov ReportBase: 64.13% // Head: 63.69% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #50 +/- ##
==========================================
- Coverage 64.13% 63.69% -0.44%
==========================================
Files 2 2
Lines 145 146 +1
Branches 19 18 -1
==========================================
Hits 93 93
- Misses 52 53 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
src/iterative_telemetry/__init__.py
Outdated
| "python_version": f"{major}.{minor}.{patch}", | ||
| "python_version_minor": f"{major}.{minor}", |
There was a problem hiding this comment.
What's the point of having both? The naming is confusing too.
There was a problem hiding this comment.
We might want to know what python implementation this is in the future, maybe worth making it a dict with misc python info right away?
There was a problem hiding this comment.
I guess minor version is what we want to plot mainly, but patch also may be valuable.
No reason to do dict since it'll be flattened in BQ
There was a problem hiding this comment.
@mike0sv BQ?
Isn't there a way to parse the version properly from major.minor.patch? Or maybe store major/minor/patch separately. Having the same thing twice looks really odd. python_version_minor sounds like just minor to me, not major.minor. This is just really odd looking. Or maybe I'm not used to this and this is a standard practice?
There was a problem hiding this comment.
BQ = big query
Doesnt make sense to store major separately - at least until python 4 is released :)
We can store major+minor and patch separately - joining is easier than splitting
There was a problem hiding this comment.
Sorry for the delay.
Storing major.minor and patch separately seems equally ugly. Looking at, for example, homebrew analytics, seems like they store their versions normally without duplicated odd version slices.
Googling around, seems like grouping by split string in bq is easily doable, but maybe I'm missing some complications?
mike0sv
force-pushed
the
fearure/python-version
branch
from
0670017 to
00316d5
Compare
closes #47