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

Moved the QC assets into the spinalcordtoolbox package #1311

Merged
merged 7 commits into from
May 20, 2017
Merged

Conversation

peristeri
Copy link
Contributor

It makes it easier to access from within the package and solved the bug when
installing SCT in a different path.

Fixes #1300

It makes it easier to access from within the package and solved the bug when
installing SCT in a different path.

Fixes #1300
@peristeri peristeri added bug category: fixes an error in the code fix:minor priority:HIGH priority:MEDIUM and removed priority:HIGH labels May 8, 2017
@peristeri peristeri added this to the Release v3.0.4 milestone May 10, 2017
@jcohenadad
Copy link
Member

I installed version: move-assets/471bcae92d78f683a2bd41a876b20096fe3a9b3d.

I ran batch_processing.sh, and then ran sct_qc -folder /Users/julien/qc_batch_processing/ and the web browser shows no entry, even though some files are created under qc_batch_processing. See below:

screen shot 2017-05-18 at 16 21 33

Regression bug fix.
Copy link
Member

@jcohenadad jcohenadad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my comment

@peristeri peristeri merged commit fbee22b into master May 20, 2017
@peristeri peristeri deleted the move-assets branch May 20, 2017 02:52
joshuacwnewton added a commit that referenced this pull request Jan 17, 2023
E402 was initially added in [#1311](#1311), but currently, it suppresses only a few warnings, all in `registration/algorithms.py`, due to [VoxelMorph's backend settings](https:/spinalcordtoolbox/spinalcordtoolbox/blob/1b5c49c3904dffe63546adbadc47759a53fbc8ca/spinalcordtoolbox/registration/algorithms.py#L24-L27).

I think we could easily convert this E402 global exclusion into a `# noqa` comment for VoxelMorph only, to allow us to still catch E402 errors.
joshuacwnewton added a commit that referenced this pull request Jan 17, 2023
* `setup.cfg`: Remove unnecessary [bdist-wheel] section

This is applied when [creating wheel files](https://stackoverflow.com/q/31573107) for a Python package. It denotes a universal wheel, i.e. one that is platform independent.

SCT does not create wheels for its `spinalcordtoolbox` package, so this is unnecessary and can be removed.

* `setup.cfg`: Remove unnecessary [aliases] section

This setting only exists in the context of a _very_ outdated copy of Pytest's documentation: https://docs.pytest.org/en/4.6.x/goodpractices.html#integrating-with-setuptools-python-setup-py-test-pytest-runner

We do not use `pytest-runner`, though, nor do we ever execute tests via `python setup.py test`. So, this can be removed.

* `setup.cfg`: Remove unnecessary [isort] section

[`isort`](https://pycqa.github.io/isort/docs/configuration/config_files.html)  is a code-formatting library, similar to `black`, that imposes a strict style on imports at the top of scripts. We don't currently install or use `isort`.

* `setup.cfg`: Remove unnecessary flake8 filetype exclusion

We don't use `.tox`. But, more importantly, we don't even run `flake8` on the root directory; we only run it on git-tracked files:

https:/spinalcordtoolbox/spinalcordtoolbox/blob/1b5c49c3904dffe63546adbadc47759a53fbc8ca/.github/workflows/lint_code.yml#L22

Since our `.gitignore` covers this already, the `exclude` setting can be removed.

* `setup.cfg`: Replace `E402` exclusion with `# noqa` comments

E402 was initially added in [#1311](#1311), but currently, it suppresses only a few warnings, all in `registration/algorithms.py`, due to [VoxelMorph's backend settings](https:/spinalcordtoolbox/spinalcordtoolbox/blob/1b5c49c3904dffe63546adbadc47759a53fbc8ca/spinalcordtoolbox/registration/algorithms.py#L24-L27).

I think we could easily convert this E402 global exclusion into a `# noqa` comment for VoxelMorph only, to allow us to still catch E402 errors.

* `setup.cfg`: Remove E501 to make `max-line-length` useful again

With E501 set, then `max-line-length` does nothing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug category: fixes an error in the code priority:HIGH
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants