ledatelescope / bifrost

A stream processing framework for high-throughput applications.
BSD 3-Clause "New" or "Revised" License
67 stars 29 forks source link

Ditch python2 #216

Closed league closed 1 year ago

league commented 1 year ago

This was a branch, but I just updated it from master and started a PR to more easily see and discuss changes.

(I was about to delete some __future__ stuff when I remembered to look for this branch.)

jaycedowell commented 1 year ago

It might be good to pick some of the "safer" things from #178 and add them in here.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 87.29% and project coverage change: +2.16% :tada:

Comparison is base (becbf48) 65.39% compared to head (f5b0b02) 67.56%. Report is 2 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #216 +/- ## ========================================== + Coverage 65.39% 67.56% +2.16% ========================================== Files 67 65 -2 Lines 5840 5515 -325 ========================================== - Hits 3819 3726 -93 + Misses 2021 1789 -232 ``` | [Files Changed](https://app.codecov.io/gh/ledatelescope/bifrost/pull/216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LEDA+Collaboration) | Coverage Δ | | |---|---|---| | [python/bifrost/\_\_init\_\_.py](https://app.codecov.io/gh/ledatelescope/bifrost/pull/216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LEDA+Collaboration#diff-cHl0aG9uL2JpZnJvc3QvX19pbml0X18ucHk=) | `79.31% <ø> (-0.69%)` | :arrow_down: | | [python/bifrost/block\_chainer.py](https://app.codecov.io/gh/ledatelescope/bifrost/pull/216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LEDA+Collaboration#diff-cHl0aG9uL2JpZnJvc3QvYmxvY2tfY2hhaW5lci5weQ==) | `96.42% <ø> (-0.13%)` | :arrow_down: | | [python/bifrost/blocks/\_\_init\_\_.py](https://app.codecov.io/gh/ledatelescope/bifrost/pull/216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LEDA+Collaboration#diff-cHl0aG9uL2JpZnJvc3QvYmxvY2tzL19faW5pdF9fLnB5) | `100.00% <ø> (ø)` | | | [python/bifrost/blocks/accumulate.py](https://app.codecov.io/gh/ledatelescope/bifrost/pull/216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LEDA+Collaboration#diff-cHl0aG9uL2JpZnJvc3QvYmxvY2tzL2FjY3VtdWxhdGUucHk=) | `71.05% <ø> (-0.75%)` | :arrow_down: | | [python/bifrost/blocks/audio.py](https://app.codecov.io/gh/ledatelescope/bifrost/pull/216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LEDA+Collaboration#diff-cHl0aG9uL2JpZnJvc3QvYmxvY2tzL2F1ZGlvLnB5) | `8.00% <ø> (-3.54%)` | :arrow_down: | | [python/bifrost/blocks/convert\_visibilities.py](https://app.codecov.io/gh/ledatelescope/bifrost/pull/216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LEDA+Collaboration#diff-cHl0aG9uL2JpZnJvc3QvYmxvY2tzL2NvbnZlcnRfdmlzaWJpbGl0aWVzLnB5) | `96.05% <ø> (-0.06%)` | :arrow_down: | | [python/bifrost/blocks/copy.py](https://app.codecov.io/gh/ledatelescope/bifrost/pull/216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LEDA+Collaboration#diff-cHl0aG9uL2JpZnJvc3QvYmxvY2tzL2NvcHkucHk=) | `100.00% <ø> (ø)` | | | [python/bifrost/blocks/correlate.py](https://app.codecov.io/gh/ledatelescope/bifrost/pull/216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LEDA+Collaboration#diff-cHl0aG9uL2JpZnJvc3QvYmxvY2tzL2NvcnJlbGF0ZS5weQ==) | `98.11% <ø> (+1.62%)` | :arrow_up: | | [python/bifrost/blocks/detect.py](https://app.codecov.io/gh/ledatelescope/bifrost/pull/216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LEDA+Collaboration#diff-cHl0aG9uL2JpZnJvc3QvYmxvY2tzL2RldGVjdC5weQ==) | `20.00% <ø> (-3.44%)` | :arrow_down: | | [python/bifrost/blocks/fdmt.py](https://app.codecov.io/gh/ledatelescope/bifrost/pull/216?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LEDA+Collaboration#diff-cHl0aG9uL2JpZnJvc3QvYmxvY2tzL2ZkbXQucHk=) | `83.33% <ø> (-0.22%)` | :arrow_down: | | ... and [53 more](https://app.codecov.io/gh/ledatelescope/bifrost/pull/216?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LEDA+Collaboration) | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

league commented 1 year ago

I think all of the clean-ups in #178 are here now, except the ones that are more complicated:

But commits in this PR (or that previously made it to master) include these clean-ups:

…in addition to the Python2 cleanups.

jaycedowell commented 1 year ago

I recall problems when I tried to remove block.py but I don't remember exactly what they were. It's a shame that those logs from #178 have expired and been removed.

jaycedowell commented 1 year ago

When I was working on 8cff5c2 I noticed that there was another Python2-ism buried in test_scripts.py. There might be other things labeled as "Python2 catch" in the code, specifically related to bytes -> str -> bytes conversion that were missed.

jaycedowell commented 1 year ago

Should we put a "needs Python 3.6+" message somewhere? That's the only thing I can thing of that this might be missing.

Added in b5f503a.

jaycedowell commented 1 year ago

Should we also change configure to check if Python is >=3.6?

Add in e01355f.

jaycedowell commented 1 year ago

That's everything now.