Closed yimuchen closed 1 year ago
Thanks, this looks useful! I've triggered the tests, but we should have tests as you also point out to make sure this is working and will continue to work in the future. I'll wait for you to add some. Let me know if you need help.
Cool! If the developers think this contribution is in the right direction, I will continue rounding out the features, testing and the linting! More to come over the next few days.
@clelange I have included all the features that for testing. Also checked with against pylint
for the new hist_utils.py
.
@yimuchen : can you please also check the linting of tests/test_histread.py
? The CI runs python -m pylint tests/*.py --rcfile=tests/pylintrc
which is currently failing.
Hm... I just checked, the hist
package is only strictly available for python>=3.7
[1], so that is why some of the CI scripts are failing. I'm not sure if there is an easy way to limit sub-packages to require a different python version.
Hm... I just checked, the
hist
package is only strictly available forpython>=3.7
[1], so that is why some of the CI scripts are failing. I'm not sure if there is an easy way to limit sub-packages to require a different python version.
Yes, there's a way, but I need to look it up. We could either deprecate Python 3.6 since it's EOL anyway or do that. I'll get back to you later (hopefully tomorrow) in case nobody else has done so in the meantime.
Merging #243 (11ae275) into main (8dcf6a1) will decrease coverage by
0.51%
. The diff coverage is81.18%
.
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
@@ Coverage Diff @@
## main #243 +/- ##
==========================================
- Coverage 88.54% 88.04% -0.51%
==========================================
Files 4 5 +1
Lines 978 1079 +101
Branches 203 229 +26
==========================================
+ Hits 866 950 +84
- Misses 82 91 +9
- Partials 30 38 +8
Flag | Coverage Δ | |
---|---|---|
unittests-3.10 | 88.04% <81.18%> (-0.51%) |
:arrow_down: |
unittests-3.6 | 87.73% <81.18%> (-0.48%) |
:arrow_down: |
unittests-3.7 | 87.73% <81.18%> (-0.48%) |
:arrow_down: |
unittests-3.8 | 87.85% <81.18%> (-0.49%) |
:arrow_down: |
unittests-3.9 | 87.85% <81.18%> (-0.49%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
Files | Coverage Δ | |
---|---|---|
hepdata_lib/hist_utils.py | 81.18% <81.18%> (ø) |
Hm... I just checked, the
hist
package is only strictly available forpython>=3.7
[1], so that is why some of the CI scripts are failing. I'm not sure if there is an easy way to limit sub-packages to require a different python version. [1] https://pypi.org/project/hist/Yes, there's a way, but I need to look it up. We could either deprecate Python 3.6 since it's EOL anyway or do that. I'll get back to you later (hopefully tomorrow) in case nobody else has done so in the meantime.
Thinking about this a bit more, I would say we should just get rid of Python 3.6 support. It's not great since lots of systems we use still have 3.6 as system Python, but then again there are relatively easy ways to get to Python >= 3.7 and we need to make sure we document them.
Hm... I just checked, the
hist
package is only strictly available forpython>=3.7
[1], so that is why some of the CI scripts are failing. I'm not sure if there is an easy way to limit sub-packages to require a different python version.
The last version of hist
for Python 3.6 is v2.4.0. The tests are failing for Python 3.6 not because hist
cannot be installed, but because you're using features from hist
that were not available in v2.4.0. So it might be possible to keep Python 3.6 support if you rewrite your code to work with hist
v2.4.0. But if this is difficult or you need features from the later hist
versions, then dropping Python 3.6 support is definitely the simpler solution.
Let me quickly compare what are the minimum set of features required for hist==2.4.0
This new readout method should work with both older versions of hist, as well as user defined accumulators.
:books: Documentation preview :books:: https://hepdata-lib--243.org.readthedocs.build/en/243/