HomePhabricator
Diffusion MITK 2f1b90756f29

Move project from PythonPackageExample to PythonPackage. Add example console…
Audit Required2f1b90756f29

Description

Move project from PythonPackageExample to PythonPackage. Add example console script as entry point (see setup.py and bin/samplepackage_exe) Use python 3 style print functions.

Details

Auditors
floca
nolden
Provenance
wirkertAuthored on Sep 30 2016, 10:59 AM
wirkertPushed on Sep 30 2016, 10:59 AM
Parents
rMITK2ce0cfc1cb6f: deleted accidentally checked in file.
Branches
Unknown
Tags
Unknown

Event Timeline

This is an updated version of my PythonPackage proposal. I moved it from PythonPackageExample to PythonPackage (as it was already in folder Examples).
Furthermore, I added a script to show how to use entry points to create console applications.
Some further small changes: 1. removed accidentally checked in files. 2. use python3 style print functions.

Happy for your comments.

Note that the comment for the "deprecated" audit is still valid:

First version of python example package ready for audit.

Here we created a samplepackage based on recommendations from https://github.com/kennethreitz/samplemod/tree/master/

The proposed structure captures all package code in the samplepackage subfolder. Additionally, ipython notebook tutorials lie in the tutorials subfolder. A doc folder was created to comply with MITK documentation guidelines.
In the future, this might be switched from a doxygen documentation to a more
python compatible implementation (like sphinx).

README, Makefile, setup.py and other important files are located in the root directory.

Open issues are in my opinion:

  1. Continous integration, possibly with tox and jenkins
  2. Documentation with doxygen is currently suboptimal

So to get it right. The code content of your old audit is deperecated, but not the comment. So one should keep the comments in mind, if one does this audit. Correct?

On the first glance, I liked it. Definetly the right direction. Thanks for the effort, will come in handy for our reorganisation of AVID too. :)
Just one comment so far. I will look closer, when I got more time in the next days.

/Examples/PythonPackage/requirements.txt
1

I think it is not a good idea to build up a requirement to nose bekause it is likely to be deprecated soon (see note from the nose website below). May be a dump question, but why note just keep with unittest? Simple but less dependencies.

cited from http://nose.readthedocs.io/en/latest/:

Note to Users
Nose has been in maintenance mode for the past several years and will likely cease without a new person/team to take over maintainership. New projects should consider using Nose2, py.test, or just plain unittest/unittest2.

floca raised a concern with this commit.Sep 30 2016, 2:13 PM

oops, chosed the wrong action.

Hi Ralf,

exactly. But I copy pasted the comment from the previous audit here, so actually just forget about it. I just can't delete the old audit. Maybe you need to "accept" it to get rid of it.

Thanks for auditing!

/Examples/PythonPackage/requirements.txt
1

I think you're right. If I understand correctly, unittest, the package used in this example project, is available without installing anything particular anyways. However, it would still be nice to have a requirement here, so the file is not empty. Maybe commented out like in setup.py:

install_requires=[],
# a more sophisticated project might have something like:
# install_requires=['numpy>=1.11.0', 'scipy>=0.17', 'scikit-learn']

so this would be something like:

// comment out to add these to requirements.txt
// scipy = 0.17
// numpy=1.10.0

However, I would need to look up if this is the correct way to comment in requirements file :-)

floca raised a concern with this commit.EditedOct 10 2016, 11:24 PM

So finally (sorry for the dealy). Following general questions/concerns:

  1. I am not sure, but i think the folder name "PythonPackage" is missleading, isn't it? From a python point of view it is a repository/group of packages conforming one projekt/deployment unit because the have the same testing and setup entry points. So I would avoid the termi package on this level ("Samplepackage" below is fine). May be just "PythonProject"?
  2. For a standalone projekt this would be totaly fine. But as it is in the MITK source tree. Have you given a thought about both, MITK and Python packages, should be organized together? I think a comment/advice on this topic would also be usefull, don't you? I don't think this is a stopper for this commit. More a candidate for a new task, that could be addressed on a friday bugsquashing to advance the whole topic. For standalone projects I think the structure is fine. In conjunction with MITK questions that would pop up in my mind are:
    1. Where should I put my Python packages/modules?
    2. Is a python package equivalent to a MITK module (so in the modules folder)?
    3. Does it need a seperate top level folder?
    4. Are mixed modules allowed, so having classic c++ but also serving as a python package? Is the layout different?
    5. Would be the python test and setup entry point then even more top level (so the MITK root) so you setup MITK as a python package collection?
    6. What are the use cases? (May be Ralf is just over designing ;) But in conjunction with (hopefully) expanding python support in MITK i think it makes sence to have in python something like "from mitk.dicomreader import autoselector" or "import mitk.core.ioutils" and then have access to bindings in "supportive" modules.

Please don't get my concerns wrong. For stand alones I find it already very usefull and it will serve well for our restructuring of AVID ;).

/Examples/PythonPackage/requirements.txt
1

Ok, I think this is a good idea. I am on the same page. I would not leave an empty file. Either use a unproplematic requirement like numpy or the proposed "comment option".

Hi Ralf,

thank you for these valid points. I started this project with a standalone python package in mind. Since back then it was not possible to have separate repositories we put it in the examples folder. I personally would favour making it it's own repository as an example of how python packages can look like.

All the points you raised concerning MITK/Python are also relevant. Not so much for me, since I use both separately but maybe for others in the department. Should we discuss this in today’s MITK-Sitzung?

Best
Sebastian

I think we should proceed like you proposed: Bringing Point 2 into the MITK-meeting and most likely convert it into a Task/BugSquashingProject.
Regarding Point 1, also (or even ?!?) in a stand alone project I think that "PythonProjectTemplate" or "ExamplePythonProject" would be a better folder name then "PythonPackage".

floca resigned from this commit.