thompson318 commented 2 years ago

@mxochicale , as your first task can you go through this tutorial yourself. Main aim is to learn some unit testing and github actions. Secondary aim is to find any bits of the tutorial that need updating and either fix them or let me know about them.

mxochicale commented 2 years ago

Thanks @thompson318 for raising this one. See below sections presenting progress and miscellaneous.

Progress

[x] Create conda env conda activate scikit-surgery-VE with basic packages for scikit-surgerytutorial01

name: scikit-surgery-VE
channels:
- defaults
- conda-forge #vtk; tox; notebook; jupyter_contrib_nbextensions; tqdm
- anaconda #scipy; ipython
dependencies:
- python=3.7
- cookiecutter>=1.7.3
- numpy>=1.21.5
- scipy>=1.7.3
- tox>=3.26.0
- pytest>=7.1.2
- pylint>=2.14.5
- vtk>=9.0.3
- pip>=22.2.2
- pip:
 - scikit-surgeryvtk>=1.0.6
 - scikit-surgeryimage>=0.10.1
 - scikit-surgerycore>=0.6.10
 - pandoc>=2.2

If it is helpful, I am happy to incorporate the above yml file with instructions in the tutorial as it helps users to use same packages and its versions.

[x] Solving authentificaiton issues

mkdir -p ~/repositories/SciKit-Surgery && cd ~/repositories/SciKit-Surgery
conda activate scikit-surgery-VE
cookiecutter https://github.com/SciKit-Surgery/PythonTemplate.git

 git push origin master
Username for 'https://github.com': mxochicale
Password for 'https://mxochicale@github.com': 
remote: Support for password authentication was removed on August 13, 2021.
remote: Please see https://docs.github.com/en/get-started/getting-started-with-git/about-remote-repositories#cloning-with-https-urls for information on currently recommended modes of authentication.
fatal: Authentication failed for 'https://github.com/mxochicale/scikit-surgery-sphere-fitting.git/'

(scikit-surgery-VE) miguel@smartliveri4i-Alienware-13-R3:~/repositories/SciKit-Surgery/sksurgeryspherefitting$ git push origin master
Username for 'https://github.com': mxochicale
Password for 'https://mxochicale@github.com': 
Counting objects: 48, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (42/42), done.
Writing objects: 100% (48/48), 43.73 KiB | 6.25 MiB/s, done.
Total 48 (delta 1), reused 0 (delta 0)
remote: Resolving deltas: 100% (1/1), done.
To https://github.com/mxochicale/scikit-surgery-sphere-fitting.git
 ! [remote rejected] master -> master (refusing to allow a Personal Access Token to create or update workflow `.github/workflows/ci.yml` without `workflow` scope)
(scikit-surgery-VE) miguel@smartliveri4i-Alienware-13-R3:~/repositories/SciKit-Surgery/sksurgeryspherefitting$ git push origin masterUsername for 'https://github.com': mxochicale
Password for 'https://mxochicale@github.com':
Counting objects: 48, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (42/42), done.
Writing objects: 100% (48/48), 43.73 KiB | 7.29 MiB/s, done.
Total 48 (delta 1), reused 0 (delta 0)
remote: Resolving deltas: 100% (1/1), done.
To https://github.com/mxochicale/scikit-surgery-sphere-fitting.git
 ! [remote rejected] master -> master (refusing to allow a Personal Access Token to create or update workflow `.github/workflows/ci.yml` without `workflow` scope)
error: failed to push some refs to 'https://github.com/mxochicale/scikit-surgery-sphere-fitting.git'

The above was sorted out following this doc Settings>Developer Settings>Personal Tokens; activating

all in repo
workflow "update github Action workflows"

Alternatively, one can use "Instead of manually entering your PAT for every HTTPS Git operation, you can cache your PAT with a Git client." For more information, see "Caching your GitHub credentials in Git."

[x] passed Testing tox and tox -r but issues with coverage:


miguel@smartliveri4i-Alienware-13-R3:~$ cd repositories/SciKit-Surgery/scikit-surgery-sphere-fitting/
(scikit-surgery-VE) miguel@smartliveri4i-Alienware-13-R3:~/repositories/SciKit-Surgery/scikit-surgery-sphere-fitting$ conda activate scikit-surgery-VE

(scikit-surgery-VE) miguel@smartliveri4i-Alienware-13-R3:~/repositories/SciKit-Surgery/scikit-surgery-sphere-fitting$ tox -r py37 recreate: /home/miguel/repositories/SciKit-Surgery/scikit-surgery-sphere-fitting/.tox/py37 py37 installdeps: -rrequirements-dev.txt py37 installed: alabaster==0.7.12,altgraph==0.17.3,astroid==2.12.11,attrs==22.1.0,Babel==2.10.3,backcall==0.2.0,beautifulsoup4==4.11.1,bleach==5.0.1,certifi==2022.9.24,charset-normalizer==2.1.1,coverage==6.5.0,debugpy==1.6.3,decorator==5.1.1,defusedxml==0.7.1,dill==0.3.5.1,distlib==0.3.6,docutils==0.17.1,entrypoints==0.4,fastjsonschema==2.16.2,filelock==3.8.0,idna==3.4,imagesize==1.4.1,importlib-metadata==5.0.0,importlib-resources==5.10.0,iniconfig==1.1.1,ipykernel==6.16.0,ipython==7.34.0,isort==5.10.1,jedi==0.18.1,Jinja2==3.1.2,jsonschema==4.16.0,jupyter-core==4.11.1,jupyter_client==7.4.2,jupyterlab-pygments==0.2.2,lazy-object-proxy==1.7.1,MarkupSafe==2.1.1,matplotlib-inline==0.1.6,mccabe==0.7.0,mistune==2.0.4,mock==4.0.3,nbclient==0.7.0,nbconvert==7.2.1,nbformat==5.7.0,nbsphinx==0.8.9,nest-asyncio==1.5.6,numpy==1.21.6,packaging==21.3,pandocfilters==1.5.0,parameterized==0.8.1,parso==0.8.3,pexpect==4.8.0,pickleshare==0.7.5,pkgutil_resolve_name==1.3.10,platformdirs==2.5.2,pluggy==1.0.0,prompt-toolkit==3.0.31,psutil==5.9.2,ptyprocess==0.7.0,py==1.11.0,pyfakefs==5.0.0,Pygments==2.13.0,pyinstaller==5.5,pyinstaller-hooks-contrib==2022.10,pylint==2.15.4,pyparsing==3.0.9,pyrsistent==0.18.1,pytest==7.1.3,python-dateutil==2.8.2,pytz==2022.4,pyzmq==24.0.1,requests==2.28.1,scipy==1.7.3,six==1.16.0,snowballstemmer==2.2.0,soupsieve==2.3.2.post1,Sphinx==5.3.0,sphinx-rtd-theme==1.0.0,sphinxcontrib-applehelp==1.0.2,sphinxcontrib-devhelp==1.0.2,sphinxcontrib-htmlhelp==2.0.0,sphinxcontrib-jsmath==1.0.1,sphinxcontrib-qthelp==1.0.3,sphinxcontrib-serializinghtml==1.1.5,tinycss2==1.1.1,tomli==2.0.1,tomlkit==0.11.5,tornado==6.2,tox==3.26.0,traitlets==5.4.0,typed-ast==1.5.4,typing_extensions==4.4.0,urllib3==1.26.12,virtualenv==20.16.5,wcwidth==0.2.5,webencodings==0.5.1,wrapt==1.14.1,zipp==3.9.0 py37 run-test-pre: PYTHONHASHSEED='4019250281' py37 run-test-pre: commands[0] | ipython kernel install --user --name=scikit-surgery-sphere-fitting Installed kernelspec scikit-surgery-sphere-fitting in /home/miguel/.local/share/jupyter/kernels/scikit-surgery-sphere-fitting py37 run-test: commands[0] | coverage erase py37 run-test: commands[1] | coverage run -a --source ./scikit-surgery-sphere-fitting -m pytest -v -s ./tests/ ======================================================== test session starts ======================================================== platform linux -- Python 3.7.13, pytest-7.1.3, pluggy-1.0.0 -- /home/miguel/repositories/SciKit-Surgery/scikit-surgery-sphere-fitting/.tox/py37/bin/python cachedir: .tox/py37/.pytest_cache rootdir: /home/miguel/repositories/SciKit-Surgery/scikit-surgery-sphere-fitting plugins: pyfakefs-5.0.0 collected 2 items

tests/test_requirements.py::test_requirements_vs_setup PASSED tests/test_scikit-surgery-sphere-fitting.py::test_fit_sphere_least_squares PASSED

========================================================= 2 passed in 1.13s ========================================================= /home/miguel/repositories/SciKit-Surgery/scikit-surgery-sphere-fitting/.tox/py37/lib/python3.7/site-packages/coverage/inorout.py:519: CoverageWarning: Module ./scikit-surgery-sphere-fitting was never imported. (module-not-imported) self.warn(f"Module {pkg} was never imported.", slug="module-not-imported") /home/miguel/repositories/SciKit-Surgery/scikit-surgery-sphere-fitting/.tox/py37/lib/python3.7/site-packages/coverage/control.py:801: CoverageWarning: No data was collected. (no-data-collected) self._warn("No data was collected.", slug="no-data-collected") py37 run-test: commands[2] | coverage report -m No data to report. ERROR: InvocationError for command /home/miguel/repositories/SciKit-Surgery/scikit-surgery-sphere-fitting/.tox/py37/bin/coverage report -m (exited with code 1) lint recreate: /home/miguel/repositories/SciKit-Surgery/scikit-surgery-sphere-fitting/.tox/lint lint installdeps: pylint, -rrequirements-dev.txt


* [x] Testing `pylint`

* modified /tests/pylintrc   `# Maximum number of characters on a single line. max-line-length=80` to  `# Maximum number of characters on a single line. max-line-length=100`

(scikit-surgery-VE) miguel@smartliveri4i-Alienware-13-R3:~/repositories/SciKit-Surgery/scikit-surgery-sphere-fitting$ pylint --rcfile=tests/pylintrc scikitsurgeryspherefitting ***** Module tests/pylintrc tests/pylintrc:1:0: E0015: Unrecognized option found: optimize-ast, files-output, variable-name-hint, class-attribute-name-hint, argument-name-hint, module-name-hint, const-name-hint, inlinevar-name-hint, method-name-hint, function-name-hint, attr-name-hint, class-name-hint, no-space-check (unrecognized-option)

...

Report

20 statements analysed.

Statistics by type

+---------+-------+-----------+-----------+------------+---------+ |type |number |old number |difference |%documented |%badname | +=========+=======+===========+===========+============+=========+ |module |4 |4 |= |100.00 |0.00 | +---------+-------+-----------+-----------+------------+---------+ |class |0 |NC |NC |0 |0 | +---------+-------+-----------+-----------+------------+---------+ |method |0 |NC |NC |0 |0 | +---------+-------+-----------+-----------+------------+---------+ |function |2 |2 |= |100.00 |0.00 | +---------+-------+-----------+-----------+------------+---------+

78 lines have been analyzed

Raw metrics

+----------+-------+------+---------+-----------+ |type |number |% |previous |difference | +==========+=======+======+=========+===========+ |code |29 |37.18 |29 |= | +----------+-------+------+---------+-----------+ |docstring |24 |30.77 |24 |= | +----------+-------+------+---------+-----------+ |comment |11 |14.10 |11 |= | +----------+-------+------+---------+-----------+ |empty |14 |17.95 |14 |= | +----------+-------+------+---------+-----------+

Duplication

+-------------------------+------+---------+-----------+ | |now |previous |difference | +=========================+======+=========+===========+ |nb duplicated lines |0 |0 |0 | +-------------------------+------+---------+-----------+ |percent duplicated lines |0.000 |0.000 |= | +-------------------------+------+---------+-----------+

Messages by category

+-----------+-------+---------+-----------+ |type |number |previous |difference | +===========+=======+=========+===========+ |convention |0 |0 |0 | +-----------+-------+---------+-----------+ |refactor |0 |0 |0 | +-----------+-------+---------+-----------+ |warning |0 |0 |0 | +-----------+-------+---------+-----------+ |error |0 |0 |0 | +-----------+-------+---------+-----------+

Messages

+-----------+------------+ |message id |occurrences | +===========+============+

Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

mxochicale commented 2 years ago

Miscellaneous

List of items to update in a new branches

Add hyperlink for body of evidence in "There is a substantial body of evidence that thinking before coding leads to better code."
git commit -m "Issue #1 added some functional requirements" in https://scikit-surgerytutorial02.readthedocs.io/en/latest/04_Planning.html might be simplified using git commit -m "added some functional requirements (#1)"
pylint --rcfile=tests/pylintrc scikitsurgeryspherefitting is not mentioned in https://scikit-surgerytutorial02.readthedocs.io/en/latest/06.1_Linting.html which might be good to incorporate
Do we need .gitlab-ci.yml?
It is a bit confusion to me to have project_name [My New Project]: scikit-surgery-sphere-fitting and project_slug [scikit-surgery-sphere-fitting]: sksurgeryspherefitting at https://scikit-surgerytutorial02.readthedocs.io/en/latest/01_Starting_Out.html, also repository path, license and UCL license should be added in the docs.
Figures are from weiss gitlab which require to be updated to github : https://scikit-surgerytutorial02.readthedocs.io/en/latest/01_Starting_Out.html
tests/test_sksurgeryspherefitting_demo.py https://scikit-surgerytutorial02.readthedocs.io/en/latest/07_UserInterface.html is missing a tab, the following one should work:
```
# coding=utf-8
```

"""scikit-surgery-sphere-fitting tests"""

from sksurgeryspherefitting.ui.sksurgeryspherefitting_demo import run_demo

def test_fit_sphere_least_squares_demo(): model_name = 'data/CT_Level_1.vtp' output_name = 'out_temp.vtp'

run_demo(model_name, output_name)

* Command test that is useful for me: `pylint --rcfile=tests/pylintrc scikitsurgeryspherefitting`
* `source .tox/py36/bin/activate` might be not necesary when using conda env covering all dependencies
* Not sure how to run vtk viewer, just managed:

wget https://github.com/thompson318/scikit-surgery-sphere-fitting/raw/master/data/US_Sphere_2.vtp

(scikit-surgery-VE) miguel@smartliveri4i-Alienware-13-R3:~/repositories/SciKit-Surgery/scikit-surgery-sphere-fitting$ python sksurgeryspherefitting.py data/CT_Level_1.vtp -o sphere.vtp Result is (array([136.571217 , 151.97335771, -95.51789211, 8.11853981]), 2) (scikit-surgery-VE) miguel@smartliveri4i-Alienware-13-R3:~/repositories/SciKit-Surgery/scikit-surgery-sphere-fitting$ python sksurgeryspherefitting.py data/US_Sphere_2.vtp -o sphere.vtp Result is (array([252.96783239, 216.57631804, 270.37878929, 596.29375964]), 2)


* Do I need `Pandoc wasn't found.` for tox -e docs
  * `scikit-surgery-VE; pip pandoc` don't work
  * in the terminal `sudo apt install pandoc`  > `docs: commands succeeded congratulations :)`
* Incomplete install_requires in `. Open it and edit it. The most important thing to do is to update the “install_requires" ` https://scikit-surgerytutorial02.readthedocs.io/en/latest/09_Releaseing.html

mxochicale commented 2 years ago

Tutorial has been completed with the "First release" https://github.com/mxochicale/scikit-surgery-sphere-fitting/releases/tag/v0.0.1

Other minor bits that might be addressed:

[x] check readthedocs and webhook
[x] ci relase in github as current version seems to be documented for gitlab $PYPI_USER and $PYPI_PAS https://scikit-surgerytutorial02.readthedocs.io/en/latest/09_Releaseing.html. See https://github.com/thompson318/scikit-surgery-sphere-fitting/blob/master/.github/workflows/ci.yml

mxochicale commented 1 year ago

Hi @thompson318

I am closing this one as it has been addressed with the replication of https://github.com/mxochicale/sksurgeryspherefitting adding few logs in the issues on packages versions that can be considered for coming or future work: https://github.com/mxochicale/sksurgeryspherefitting/issues/2

Thanks

SciKit-Surgery / scikit-surgerytutorial02

Miguel review #25

Progress

Report

Statistics by type

Raw metrics

Duplication

Messages by category

Messages

Miscellaneous