tee-ar-ex / trx-python

Python implementation of the TRX file format
https://tee-ar-ex.github.io/trx-python/
BSD 2-Clause "Simplified" License
20 stars 16 forks source link

Add an installation from a specific fork/commit. #57

Closed arokem closed 1 year ago

arokem commented 1 year ago

Just a demo, so we can report this upstream to the dipy maintainers (since they seem incredulous https://github.com/dipy/dipy/pull/2759#issuecomment-1517932962).

skoudoro commented 1 year ago

this error https://github.com/tee-ar-ex/trx-python/actions/runs/4766035647/jobs/8472529136?pr=57#step:4:639

skoudoro commented 1 year ago

ok, I see, since we do not have a pyproject.toml

it runs:

      running bdist_wheel
      running build
      running build_py
      running build_ext

but before that, it seems it does not install any DIPY dependency like cython, nibabel, numpy, etc... for now, you might have to do it manually until https://github.com/dipy/dipy/pull/2715. is done and merged

skoudoro commented 1 year ago

I think, for now, the temporary solution here, is to add an extra step to install cython. see below in install dependencies section:

    steps:
    - uses: actions/checkout@v2
    - name: Set up Python ${{ matrix.python-version }}
      uses: actions/setup-python@v2
      with:
        python-version: ${{ matrix.python-version }}
    - name: Install dependencies
      run: |
        python -m pip install --upgrade pip
        pip install cython, nibabel, numpy, scipy
        pip install .[doc]
    - name: Build docs
      run: |
        cd docs
        make html
    - name: Upload docs
      uses: actions/upload-artifact@v1
      with:
        name: docs
        path: docs/_build/html
    - name: Publish docs to Github Pages
      if: startsWith(github.event.ref, 'refs/tags')
      uses: JamesIves/github-pages-deploy-action@v4
      with:
        branch: gh-pages
        folder: docs/_build/html
arokem commented 1 year ago

Thanks Serge! I'll try adding them one by one, starting with Cython, and see whether we need also the others.

On Fri, Apr 21, 2023 at 8:35 AM Serge Koudoro @.***> wrote:

I think, for now, the temporary solution here, is to add an extra step to install cython. see below in install dependencies section:

steps:
- uses: ***@***.***
- name: Set up Python ${{ matrix.python-version }}
  uses: ***@***.***
  with:
    python-version: ${{ matrix.python-version }}
- name: Install dependencies
  run: |
    python -m pip install --upgrade pip
    pip install cython, nibabel, numpy, scipy
    pip install .[doc]
- name: Build docs
  run: |
    cd docs
    make html
- name: Upload docs
  uses: ***@***.***
  with:
    name: docs
    path: docs/_build/html
- name: Publish docs to Github Pages
  if: startsWith(github.event.ref, 'refs/tags')
  uses: ***@***.***
  with:
    branch: gh-pages
    folder: docs/_build/html

— Reply to this email directly, view it on GitHub https://github.com/tee-ar-ex/trx-python/pull/57#issuecomment-1518009233, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA46NUTZL2F6HW5WVXAV5DXCKSLRANCNFSM6AAAAAAXG6WCCY . You are receiving this because you authored the thread.Message ID: @.***>

skoudoro commented 1 year ago

some progress with your fork, it find cython but not packaging,

arokem commented 1 year ago

That's why I added the two sanity checks:

https://github.com/tee-ar-ex/trx-python/actions/runs/4769285925/jobs/8479505319?pr=57#step:5:1

and:

https://github.com/tee-ar-ex/trx-python/actions/runs/4769285925/jobs/8479505319?pr=57#step:6:1

Python finds these two installed, but when it runs pip to install dipy, they are no where to be found. So strange.

arokem commented 1 year ago

I am not sure the links in my previous message work as intended. But look at the two steps before DIPY is installed ("Did we get cython in there?" and "Did we get packaging in there?"). They are both installed and importable on the CI runner, but then seem to disappear when they are needed :-/

skoudoro commented 1 year ago

I agree, so strange... I saw those sanity check. I do not succed to go to your fork, is it the one with pyproject.toml and your PR?

skoudoro commented 1 year ago

can you add packaging in this pyproject.toml from your fork

arokem commented 1 year ago

Oh yeah - good idea. Let's see how that works...

arokem commented 1 year ago

OK - one more try with both packaging and cython in the pyproject.

arokem commented 1 year ago

OK. I get the picture. Maybe we need to add all of dipy's dependencies into the pyproject file.

skoudoro commented 1 year ago

So it confirms that project with pyproject.toml needs to have dependencies that use pyproject.toml also.

that's strange but good to know

skoudoro commented 1 year ago

yeah! all green !

arokem commented 1 year ago

Yeah! 🎉

@frheault : does this all make sense to you? To make this work, you would need to add this to your fork/branch on dipy: https://github.com/dipy/dipy/pull/2787/files, and add fury to the dependencies on the trx side.

frheault commented 1 year ago

This makes sense!

So I will go ahead and merge this PR, update my Dipy branch to have that new commit from Dipy and from there do some testing