ledatelescope / bifrost

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

Remove the deprecated numpy dtypes "int" and "float" #217

Closed jaycedowell closed 10 months ago

jaycedowell commented 11 months ago

It looks like this has sorta kinda already been merged in but with numpy.int64 replacing numpy.int and numpy.float64 replacing numpy.float. I think those are the correct sizes for native Python int's and float's so maybe this PR isn't needed? I guess the advantage of this approach is that integer allows both int32 and int64, and floating worth with float32 and float64.

jaycedowell commented 10 months ago

@dentalfloss1 @league Any opinions on this PR which would allow lower precision values to be stored in a "standard header"? I'm currently leaning towards keeping the current behavior in master and not merging this one.

dentalfloss1 commented 10 months ago

I agree.

codecov[bot] commented 10 months ago

Codecov Report

Patch and project coverage have no change.

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

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #217 +/- ## ======================================= Coverage 65.39% 65.39% ======================================= Files 67 67 Lines 5840 5840 ======================================= Hits 3819 3819 Misses 2021 2021 ``` | [Files Changed](https://app.codecov.io/gh/ledatelescope/bifrost/pull/217?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LEDA+Collaboration) | Coverage Δ | | |---|---|---| | [python/bifrost/header\_standard.py](https://app.codecov.io/gh/ledatelescope/bifrost/pull/217?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=LEDA+Collaboration#diff-cHl0aG9uL2JpZnJvc3QvaGVhZGVyX3N0YW5kYXJkLnB5) | `100.00% <ø> (ø)` | |

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

jaycedowell commented 10 months ago

Motion carries, closing.