Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code Pruning #178

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft

Code Pruning #178

wants to merge 13 commits into from

Conversation

jaycedowell
Copy link
Collaborator

This PR cleans up the code base by removing unused and commented out code. It may also be an appropriate place to address #156.

@codecov-commenter
Copy link

codecov-commenter commented Apr 29, 2022

Codecov Report

Merging #178 (e6bed74) into master (317d653) will increase coverage by 9.62%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #178      +/-   ##
==========================================
+ Coverage   58.37%   67.99%   +9.62%     
==========================================
  Files          67       67              
  Lines        5744     7262    +1518     
==========================================
+ Hits         3353     4938    +1585     
+ Misses       2391     2324      -67     
Impacted Files Coverage Δ
python/bifrost/ring.py 89.66% <ø> (ø)
python/bifrost/blocks/binary_io.py 93.44% <100.00%> (ø)
python/bifrost/version/__init__.py 100.00% <0.00%> (ø)
python/bifrost/libbifrost_generated.py 72.85% <0.00%> (ø)
python/bifrost/ring2.py 85.71% <0.00%> (+0.25%) ⬆️
python/bifrost/pipeline.py 84.87% <0.00%> (+1.17%) ⬆️
python/bifrost/DataType.py 66.89% <0.00%> (+2.02%) ⬆️
python/bifrost/blocks/transpose.py 96.87% <0.00%> (+3.12%) ⬆️
python/bifrost/memory.py 78.94% <0.00%> (+3.50%) ⬆️
python/bifrost/ndarray.py 82.93% <0.00%> (+9.78%) ⬆️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 317d653...e6bed74. Read the comment docs.

@league
Copy link
Collaborator

league commented May 3, 2022

I'm looking into the drop in coverage on ring.py. Would to complete/resolve the ring → ring2 replacement/merger with this branch?

@jaycedowell
Copy link
Collaborator Author

I've given up on removing bifrost.block since there seems to be a few knock on consequences to that. bifrost.ring vs bifrost.ring2 might be a similar thing but I think we need to sort out #179 before we try that.

@jaycedowell
Copy link
Collaborator Author

I'm starting to pick this up again. I wonder if this kind of cleanup makes sense to do as part of the no_python2 branch.

@jaycedowell jaycedowell mentioned this pull request Jul 20, 2023
league pushed a commit that referenced this pull request Jul 20, 2023
Cherry-picked from c2778a1
on branch apr2022-prune, PR #178
league added a commit that referenced this pull request Jul 21, 2023
This supersedes e6bed74 in
apr2022-prune branch (PR #178).

There was a new usage of dtype.bifrost2string in ndarray.py, which
wasn't part of the apr2022-prune branch.  I think (?) I worked around
it correctly using the enum.

We also discovered and fixed a NameError bug in DataType.
@jaycedowell
Copy link
Collaborator Author

This project has migrated over to #216 and the no_python2 branch. apr2022-prune should be removed once #216 is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants