The diagnostics-nme from nipraxis-fall-2022

Suggestion: ordering in the output of ``scripts/find_outliers.py``

Although this is not strictly required, comparison (e.g., between versions of your project) would be easier if the script outputs were always ordered in the same way.

Suggestion: resolve test warnings

Congratulations on the test deployment, I successfully run python -m pip install -r test_requirements.txt and subsequently:

(nipraxis-nme) oesteban@hos65851:~/workspace/nipraxis/diagnostics-NME$ pytest .
======================================================================================================================================================================================================= test session starts ========================================================================================================================================================================================================
platform linux -- Python 3.9.13, pytest-7.2.0, pluggy-1.0.0
rootdir: /data/home/oesteban/workspace/nipraxis/diagnostics-NME, configfile: pytest.ini
collected 3 items                                                                                                                                                                                                                                                                                                                                                                                                                  

findoutlie/tests/test_detectors.py .                                                                                                                                                                                                                                                                                                                                                                                         [ 33%]
findoutlie/tests/test_dvars.py .                                                                                                                                                                                                                                                                                                                                                                                             [ 66%]
findoutlie/tests/test_spm_funcs.py .                                                                                                                                                                                                                                                                                                                                                                                         [100%]

========================================================================================================================================================================================================= warnings summary =========================================================================================================================================================================================================
findoutlie/tests/test_detectors.py::test_iqr_detector
findoutlie/tests/test_detectors.py::test_iqr_detector
  /data/home/oesteban/workspace/nipraxis/diagnostics-NME/findoutlie/detectors.py:121: DeprecationWarning: the `interpolation=` argument to percentile was renamed to `method=`, which has additional options.
  Users of the modes 'nearest', 'lower', 'higher', or 'midpoint' are encouraged to review the method they used. (Deprecated NumPy 1.22)
    Q1 = np.percentile(measures, 25, interpolation="midpoint")

findoutlie/tests/test_detectors.py::test_iqr_detector
findoutlie/tests/test_detectors.py::test_iqr_detector
  /data/home/oesteban/workspace/nipraxis/diagnostics-NME/findoutlie/detectors.py:122: DeprecationWarning: the `interpolation=` argument to percentile was renamed to `method=`, which has additional options.
  Users of the modes 'nearest', 'lower', 'higher', or 'midpoint' are encouraged to review the method they used. (Deprecated NumPy 1.22)
    Q2 = np.percentile(measures, 50, interpolation="midpoint")

findoutlie/tests/test_detectors.py::test_iqr_detector
findoutlie/tests/test_detectors.py::test_iqr_detector
  /data/home/oesteban/workspace/nipraxis/diagnostics-NME/findoutlie/detectors.py:123: DeprecationWarning: the `interpolation=` argument to percentile was renamed to `method=`, which has additional options.
  Users of the modes 'nearest', 'lower', 'higher', or 'midpoint' are encouraged to review the method they used. (Deprecated NumPy 1.22)
    Q3 = np.percentile(measures, 75, interpolation="midpoint")

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
================================================================================================================================================================================================== 3 passed, 6 warnings in 4.40s ===================================================================================================================================================================================================

As you can see, there are three deprecations (all in the same function). Ensuring you address deprecation in time will save maintenance time in the long term.

Request: run ``black .`` to standardize the coding style

Although not strictly required for the project, the project's maintainability (and reviewing) would be very much improved if it sticks with standardized style agreements.

This would require (1) installing black; and (2) running it and pushing a PR with all the changes.

In general, and if you want to run the extra mile, you could think of installing it as a "pre-commit hook" (https://black.readthedocs.io/en/stable/integrations/source_version_control.html)

Ready for review

Hi, @pxr687 @eilidhmacnicol @matthew-brett @effigies @oesteban - please can you review our project?

Unclear outlier detection strategy in ``detect_outliers_mad_median_absolute_deviation_mask``

I would need some help understanding the strategy behind this method. Are you first finding outlying voxels in each volume and then of those, deciding which outliers are outlying in time?

unable to find detectors.py when running test_detectors.py

@effigies Hi, it looks like there is another issue. When I run python -m pytest .\findoutlie\tests\test_detectors.py, it gives me an error, unable to find detectors.py module. I have to manually put detectors.py module in the test folder to make it work. Is there something wrong or I am not on the right track?

Thank you

Potential problems in ``detectors.py`` derived from unused items

First,

diagnostics-NME/findoutlie/detectors.py

Line 17 in 6653c80

from scipy.stats import norm

where norm is import but unused.

Later

diagnostics-NME/findoutlie/detectors.py

Line 119 in 6653c80

Q2 = np.percentile(measures, 50, interpolation="midpoint")

Q2 is assigned but never used.

Improve docstrings to improve readability of the code

The core functions should feature a much more detailed description of what they are doing:

diagnostics-NME/findoutlie/outfind.py

Line 36 in 6653c80

""" Detect outliers given image file path 'filename'

diagnostics-NME/findoutlie/outfind.py

Line 65 in 6653c80

""" Detect outliers given image file path 'filename'

diagnostics-NME/findoutlie/outfind.py

Line 91 in 6653c80

""" Detect outliers given image file path 'filename'

diagnostics-NME/findoutlie/outfind.py

Line 128 in 6653c80

""" Detect outliers given image file path `filename`

Main branch of @Nauman702 is 12 commits behind this repository

Hi @Nauman702, I have figured out what is going on.

If you head to your fork of the repo (https://github.com/Nauman702/diagnostics-NME), you'll see that just above the code there's a message that states:

This branch is 12 commits behind nipraxis-fall-2022:main.

This means that in your group's repository (this one, nipraxis-fall-2022/diagnostics-NME) there are some updates and you have not updated your main branch with them.

Do you understand what that means?
If so, do you need a walkthrough to update your main branch?

Clean up unreachable code

Hi team,

I'm going through your project. A minor thing that would require some cleanup is at the end of the metrics.py file:

diagnostics-NME/solutions/metrics.py

Lines 41 to 44 in 6653c80

    
           # LAB(replace solution) 
        
           # This is a placeholder, replace it to write your solution. 
        
           raise NotImplementedError("Code up this function") 
        
           # LAB(end solution)

Because of the return, that code will not be executed. Therefore, it is better to just remove it.

group data-hash file

@effigies Hi, our project repo has group-00 folder having its hash file. However, when I download the data via curl it downloads group-01 data and its hash file in the folder group-01. When I run validate data py file, it gives me an error on group-00 hash file. It's working fine after deleting it. Is something wrong in there?

Thanks

Potential problems in ``validate_data.py`` derived from unused items

First,

diagnostics-NME/scripts/validate_data.py

Line 9 in 6653c80

import sys

where sys is imported but never used.

Later,

diagnostics-NME/scripts/validate_data.py

Line 83 in 6653c80

d_pth = list(data_pth.glob("**/*"))

assigns d_pth and it's never used.

Missing dependency - scikit-image

After downloading and installing, the execution of:

python scripts/find_outliers.py data

fails unless skimage is installed (thanks to @effigies for spotting this issue).

Please add the dependency to the pyproject.toml.

Then, I would suggest removing the requirements.txt file, as duplicity in responsibilities is a bad idea (cc @matthew-brett @effigies for insights on whether the presence of a requirements.txt file was suggested by us, or if there's something I'm missing).

sample outlier detector function output

@jagruti8 @mtach Hi both,
When I run my code with sample outlier detector function, I am getting the output as attached. It doesn't look right to me. Are you guys getting the same output?

Thank you

	# LAB(replace solution)
	# This is a placeholder, replace it to write your solution.
	raise NotImplementedError("Code up this function")
	# LAB(end solution)

nipraxis-fall-2022 / diagnostics-nme Goto Github PK

diagnostics-nme's People

Contributors

Watchers

Forkers

diagnostics-nme's Issues

Recommend Projects

Recommend Topics

Recommend Org