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 Cleaning #1471

Merged
merged 4 commits into from Apr 6, 2018

Conversation

Projects
None yet
3 participants
@Borda
Contributor

Borda commented Mar 21, 2018

minor code cleaning
consider adding code quality check which may improve the code style, e.g. codacy
... Codacy Badge

@Borda Borda referenced this pull request Mar 21, 2018

Closed

simplify SDR iterate #1470

@Borda Borda force-pushed the Borda:cleaning branch from c2faaa4 to abbe5d9 Mar 21, 2018

@codecov-io

This comment has been minimized.

codecov-io commented Mar 21, 2018

Codecov Report

Merging #1471 into master will increase coverage by 0.02%.
The diff coverage is 82.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1471      +/-   ##
==========================================
+ Coverage   87.49%   87.51%   +0.02%     
==========================================
  Files         241      241              
  Lines       30807    30698     -109     
  Branches     3321     3320       -1     
==========================================
- Hits        26954    26866      -88     
+ Misses       3078     3058      -20     
+ Partials      775      774       -1
Impacted Files Coverage Δ
dipy/boots/resampling.py 20.77% <ø> (+0.77%) ⬆️
dipy/sims/phantom.py 65.16% <ø> (+0.33%) ⬆️
dipy/viz/tests/test_widgets.py 74.75% <ø> (+3.84%) ⬆️
dipy/core/sphere_stats.py 59.01% <ø> (-0.36%) ⬇️
dipy/external/fsl.py 18.18% <ø> (+0.11%) ⬆️
dipy/sims/voxel.py 90.58% <ø> (-0.05%) ⬇️
dipy/tracking/local/tests/test_tracking.py 97.89% <ø> (-0.01%) ⬇️
dipy/direction/peaks.py 79.22% <ø> (-0.1%) ⬇️
dipy/denoise/tests/test_noise_estimate.py 100% <ø> (ø) ⬆️
dipy/segment/threshold.py 97.5% <ø> (-0.07%) ⬇️
... and 71 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 52ca2ae...459f727. Read the comment docs.

@Borda

This comment has been minimized.

Contributor

Borda commented Mar 21, 2018

it seems that the codecov/path is not much relevant since the corrected lines have not sometimes touch bu previous tests and now it marks as own issue...

@Borda

This comment has been minimized.

Contributor

Borda commented Apr 3, 2018

@skoudoro

Thanks for this @Borda.

  • Can you remove all unused variable/line instead of commenting them?

Otherwise, it looks good to me.

README.rst Outdated
@@ -12,6 +12,9 @@
.. image:: https://codecov.io/gh/nipy/dipy/branch/master/graph/badge.svg
:target: https://codecov.io/gh/nipy/dipy
.. image:: https://api.codacy.com/project/badge/Grade/65d6b1c8dfb74f2d8401cd213f55d710
:target: https://www.codacy.com/app/Borda/dipy?utm_source=github.com&amp;utm_medium=referral&amp;utm_content=Borda/dipy&amp;utm_campaign=Badge_Grade

This comment has been minimized.

@skoudoro

skoudoro Apr 4, 2018

Member

This badge can be created later, we need to create a specific account for DIPY. So you can remove it for the moment.
What do you think @arokem?

@Borda

This comment has been minimized.

Contributor

Borda commented Apr 4, 2018

regarding removing variables, since they are commented it would take more time to find them and remove them, on the other hand, there is more commented code even before my changes so I would rather propose to address it in another PR - removing commented code...

@Borda Borda force-pushed the Borda:cleaning branch from abbe5d9 to 98f2788 Apr 4, 2018

@skoudoro

This comment has been minimized.

Member

skoudoro commented Apr 5, 2018

Hi @Borda, It is much easier to remove them with a diff on this PR than tracking and removing them after a merge in a new PR. I do not see your point here... Can you explain to me why it would take more time to do it on this PR than another one?

@Borda Borda force-pushed the Borda:cleaning branch from 98f2788 to 22f8e0c Apr 5, 2018

@Borda Borda force-pushed the Borda:cleaning branch from 22f8e0c to 459f727 Apr 5, 2018

@Borda

This comment has been minimized.

Contributor

Borda commented Apr 5, 2018

OK, I remove it, but there is still other commented code...

@skoudoro

Thanks for this update @Borda.

LGTM so I will merge it tomorrow if there is no objection

@skoudoro skoudoro merged commit 17140cd into nipy:master Apr 6, 2018

2 of 3 checks passed

codecov/patch 82.5% of diff hit (target 87.49%)
Details
codecov/project 87.51% (+0.02%) compared to 52ca2ae
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Borda Borda deleted the Borda:cleaning branch Apr 6, 2018

ShreyasFadnavis pushed a commit to ShreyasFadnavis/dipy that referenced this pull request Sep 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment