Move to pyproject.toml #260
Conversation
|
I guess I should exclude the notebooks, and yaml files? |
|
Looking great so far! The doc building should be minor, the workflow is still trying to run setup.py, so just replace that with a default sphinx build command, see e.g. the template workflow: https://github.com/NLeSC/python-template/blob/main/%7B%7Bcookiecutter.directory_name%7D%7D/.github/workflows/documentation.yml To copy-paste (almost) that whole workflow, you also need to add a doc group under [project.optional-dependencies], see here. Btw, I think the conda package wasn't built for a while already, so not sure whether that stuff is still necessary, but were you planning on porting that still as well? If not, we can always do it later, probably not a priority right now. |
- deprecated from NumPy >=1.20
|
I'm a bit confused & unsure about the mixed conda + pip setup. Conda doesn't respect pyproject.toml at all. And to generate the docs, building and installing with ( This mixed setup leads to a lot of duplication of dependency specification; |
@drcandacemakedamoore do you want to generate the docs for the vendored libraries as well? I'm inclined towards yes, since they are not really maintained upstream. |
Ok docs build except for this really odd error. I could only find one import from and it is not even referenced in any docstring (I was expecting something like |
|
Can you ignore imports (or just that one) in the docs config? |
There was a problem hiding this comment.
Also this then has no conda build, and this needs discussion with tech lead @egpbos
Never mind about this.... |
pytest==8.1 was yanked because of wide incompatibilities. Since then there has been multiple new releases, and the '<8.1' restriction is not required.
TODO: not tested extensively
There was a problem hiding this comment.
This PR looks good to merge to me. It's possible that some former functionality will no longer work, but it's hard to tell. A coordinated moment to merge and deal with possible breakages immediately and to determine follow-up action points (for instance, for adding a CI test for conda building and installing) is advised.
|
The base tests are passing. There are a few junk notebooks like (cvasl\vendor\ComBat++\Untitled.ipynb) that can come off on a subsequent commit. However, the main issue is that you deleted setup.py, and this will make the on-tag workflow fail. There are also some issues if conda-build will be supported. I can therefore not merge this without a bit more investigation. |
…nding is the issue of passing the site parameters in autocombat. runharmonize is updated. data classes now accept several csv files to accomodate topmri
…ealized that the autocombat implementation which was refactored from the notebooks into the current structure actually runs the Combat class of comscan, not AutoCombat. For AutoCombat, we need to not provide the site designations for the datasets and let the algorithm run its own clustering method. Thus, in this commit I have renamed the AutoCombat into ComScanNeuroCombat for backward compatibility. The class AutoCombat will be implemented separately using the AutoCombat class of comscan
…Ive not given used access to the <site> parameter, as to not bypass autocombat and run neurocombat by mistake
…ackages will be installed automatically. The original RELIEF R package threw an error related to svd. The problem was they were scaling the input matrix and in some cases it would result in NaN, which subsequently lead to svd failing. A common solution to this problem is to replace the NaN values with zero. To achieve this, I've modified the RELIEF.R file from the original package and now we're using CVASK_RELIEF.R. In this one I've modified the softsvd function and copied the function estim_sigma originally belonging denoiseR so I can modify it. All this is handled in the code. For now I'm assuming the driver runharmonize.py is run from the same folder as mriharmonize.py. I'll handle the file location dependency after I'm done with the harmonizations and ml, alongside docstrings, optimization of the code, etc.
…rscript in a subprocess
…y. current version can be improved, especially at the end, but I'll push this for now in the interest of handing over the code faster for the paper
…o the driver. added summary statistics calculation funcrtion to the base data class.
… with make_topper function, now fixed. also check all the available methods for this problem
… to HarmComscanNeuroCombat
… to HarmNeuroCombat
…en for the same column each dataset contains only one value as in sex is only male in dataset A and only female in dataset B. Previously, both would be mapped to zero because each was mapped separately. Now we are using the encode_cat_features function. Driver is updated to reflect the changes
…nctions that are not directly called by user. fixed the reverse mapping function. updated the driver with the proper parameters
…the cvasl directory
…t which lead to an error every time we provided discrete_cluster_features
…eadout results in a singular matrix which crashes the harmonization process
…eadout results in a singular matrix which crashes the harmonization process
setuptools_scm)tox-condafor compatibility testingconda-buildstep is partialBuild with
python -m build .