Best Practices for Package Installation

[yw7]

I'me open here issue for this as suggested here: #46 (comment)
Taken from @NathanMolinier comment here: #46 (comment)

Question raised by @jcohenadad here: #46 (review)
What would be the best practice regarding the installation of the package ?

Currently we ask the user to do this

git clone https://github.com/neuropoly/totalspineseg.git 
pip install -e totalspineseg 

But we were wondering if instead it would be more intuitive for the user to do this:

git clone https://github.com/neuropoly/totalspineseg.git
cd totalspineseg
pip install -e .
cd ..

Note: After this installation we are creating a data folder outside of the repository and initializing variables, that's why we need to do move back with cd ...

Do you have any recommandations on this @joshuacwnewton @mguaypaq ?

[joshuacwnewton]

If totalspineseg is to be implemented into SCT (as a standalone package for now as per our SCT meeting discussion), then I would suggest publishing the package to PyPI to avoid the need for a git clone + editable installation at all. (This would likely be necessary in order for SCT to access totalspineseg's Python API.)

To do this, you can set up a GitHub Actions workflow that will automatically create a GitHub Release and simultaneously publish to PyPI. There are lots of examples of this out in the world, but here is a quick boilerplate example taken from one of my personal projects:

name: "Test and publish release"

on:
  workflow_dispatch:
    inputs:
      release_version:
        description: 'Release version number'
        required: true
  push:
    branches:
      - main
  pull_request:
    branches:
      - '*'

env:
  PY_COLORS: "1"

jobs:
  test-and-publish-release:
    runs-on: ubuntu-latest
    steps:
    - name: Checkout totalspineseg (main branch)
      uses: actions/checkout@v3

    - name: Set up Python
      uses: actions/setup-python@v4
      with:
        python-version: '3.8.x'

    - name: Install totalspineseg and its dev dependencies
      run: |
        pip install -e .[dev]

    - name: Run tests (Python API)
      run: |
        pytest testing --entry-point python-api

    # The GitHub Actions bot email was taken from: https://github.community/t/github-actions-bot-email-address/17204/6
    - name: Set bot user data for commits
      # Only set git user data if workflow is run manually. (This allows the other steps in the workflow to test PRs.)
      if: github.event_name == 'workflow_dispatch'
      run: |
        git config --global user.email "41898282+github-actions[bot]@users.noreply.github.com"
        git config --global user.name "GitHub Actions Bot"

    - name: Update pyproject.toml version (for release)
      # Only update the version number if workflow is run manually. (This allows the other steps in the workflow to test PRs.)
      if: github.event_name == 'workflow_dispatch'
      run: |
        toml set --toml-path pyproject.toml project.version "${{ github.event.inputs.release_version }}"
        git add pyproject.toml
        git commit -m "Update pyproject.toml version for ${{ github.event.inputs.release_version }}"

    - name: Build wheel/sdist
      run: python -m build

    - name: Push release changes
      # Only push the new tags if workflow is run manually. (This allows the other steps in the workflow to test PRs.)
      if: github.event_name == 'workflow_dispatch'
      run: |
        git tag ${{ github.event.inputs.release_version }}
        git push --tags

    - uses: ncipollo/release-action@v1
      # Only create release if workflow is run manually. (This allows the other steps in the workflow to test PRs.)
      if: github.event_name == 'workflow_dispatch'
      name: Create release
      id: create_release
      with:
        tag: ${{ github.event.inputs.release_version }}
        token: ${{ secrets.GITHUB_TOKEN }}
        artifacts: "dist/totalspineseg-${{ github.event.inputs.release_version }}.tar.gz,\
                    dist/totalspineseg-${{ github.event.inputs.release_version }}-py3-none-any.whl"
        draft: true

    - name: Publish distribution to PyPI
      # Only publish distribution if workflow is run manually. (This allows the other steps in the workflow to test PRs.)
      if: github.event_name == 'workflow_dispatch'
      run: twine upload dist/*.whl dist/*.tar.gz --username __token__ --password ${{ secrets.PYPI_API_TOKEN }}

I imagine some things will likely need to be tweaked (e.g. possibly building wheels for windows/macOS, as well as for multiple versions of Python, and also updating the actions versions to v4/v5 due to nodejs deprecations) but this is probably a good starting point, as it's a skeleton containing all of the main steps.

[yw7]

This looks greate!! Thank you @joshuacwnewton

Although, I think that we might need to consder implemnting totalspineseg directly in SCT as it will require less dependencies, and use the build in SCT way of open images, registration and resampling.
Also I think that most of the functions used in totalspineseg is easy to migrate into SCT

[joshuacwnewton]

consder implemnting totalspineseg directly in SCT

I had initially thought the same thing! Here is a link to the points I wrote on the corresponding SCT issue (spinalcordtoolbox/spinalcordtoolbox#4626):

• Or, should SCT absorb the project entirely, to preemptively account for the possibility of it going unmaintained in the future (and thus allowing us to smoothly take over maintenance of the project as part of SCT, with a shared set of dependencies).

But the discussion in the Sept 17 SCT meeting (link) seemed to conclude that we should start by keeping the projects separate for now:

• Discussion: maybe it’s fine to have TotalSpineSeg as a separate pip install during this development/integration phase, then absorb it completely into SCT as an eventual goal

But, we have another meeting this Tuesday (Oct 1) if you would like to join and discuss?

[yw7]

• Discussion: maybe it’s fine to have TotalSpineSeg as a separate pip install during this development/integration phase, then absorb it completely into SCT as an eventual goal

Actually, starting from this, then trasform all to SCT would be good option.

But, we have another meeting this Tuesday (Oct 1) if you would like to join and discuss?

I'm not sure I can on Tuesday, But if so I'll definitely join! Thanks!!