Bug#819016: jellyfish: Rename python bindings module name
Hello Andreas, I dropped by #debian-python yesterday in order to clarify conformance to Python Policy 3.3 as suggested during a review on the ITP [1], and the conversation derived into a suggestion about revisiting the decision we made about the package names (excerpt from the log [2]): Apr 12 17:56:45 there are 2 reported installations of python3-jellyfish on popcon - I'd rename the other python3-jellyfish now (module and binary package) and a then upload yours with python3-jellyfish name I'm wondering if you could share your thoughts on this issue and if you would be open to this change, even if it is quite a deviation from our envisaged solution? I'd be happy to move the discussion to other channels if needed, and again, thanks a lot for your patience on this issue. Best regards, [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=807432#85 [2] http://paste.debian.net/432489 -- Diego M. Rodriguez 36B3 42A9 9F2F 2CFB F79B FF9B B6C4 B901 06BC E232 signature.asc Description: Digital signature
Bug#819016: jellyfish: Rename python bindings module name
Source: jellyfish Severity: wishlist Dear Maintainer, after a discussion with Andreas Tille [1], I'm wondering if it would be possible to rename the python module of the python bindings from "jellyfish" to "dna_jellyfish", in order to avoid conflicts with an existing PyPI [2] package, which is also in the process of being packaged into Debian [3]. A short background on the nature of this change (more details can be found following he linked discussions, and I'd be happy to provide more specific pointers if needed), according to my understanding: * both packages define a "jellyfish" module. * a consensus in regards to the package names and the module names has been seeked with moderate results (the most likely being that the current DNA-jellyfish bindings should retain the "python-jellyfish" Debian package name, but the actual Python module should be renamed). * the upstream DNA-jellyfish author has already expressed his willingness to rename the Python module [4] as it is still not too widespread, although unfortunately it seems that there has not been too much movement in this regard in the upstream repository yet. I'm attaching two patches that would therefore solve this issue on the Debian end, hoping we can eventually tackle the issue at the upstream level. As for the new name, the patch uses "dna_jellyfish" since it was the first alternative suggested by upstream - I'd be happy to switch to another name if preferred. Both patches are the output of plain "git diff" against the patched Debian sources: rename-python-module-to-dna_jellyfish.patch: * setup.py: invokes swig using the "-module" argument [5] * setup.py: renames the Extension and the package name, and updates the older check to reference the new filename. * test_*.py: replaces the references to the "jellyfish" module with "dna_jellyfish". update-debian-rules-dh_clean.patch: * debian/rules: updates override_dh_clean to remove the swig generated .py file. I still need to do another round to ensure no further changes are needed: in particular, I still have to confirm that the python tests are actually executed (debuild output): --- make check-TESTS ... SKIP: tests/swig_python.sh ... running install_egg_info Writing /tmp/buildd/jellyfish-2.2.5/debian/python3-jellyfish/usr/lib/python3.5/dist-packages/dna_jellyfish-0.0.1.egg-in fo I: pybuild base:184: cd /tmp/buildd/jellyfish-2.2.5/.pybuild/pythonX.Y_3.5/build; python3.5 -m unittest discover -v -- Ran 0 tests in 0.000s OK ... --- The same "SKIP: tests/swig_python.sh" and "Ran 0 tests ..." lines also seem to be present on the current 2.2.5-1 debuild log, so it might not be an issue after all and a result of me not having gone through all the details about the testing yet. [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=806716#79 [2] https://pypi.python.org/pypi/jellyfish [3] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=806716 [4] https://lists.debian.org/debian-python/2015/12/msg00107.html [5] http://www.swig.org/Doc3.0/SWIGDocumentation.html#SWIG_nn2 -- Diego M. Rodriguez 36B3 42A9 9F2F 2CFB F79B FF9B B6C4 B901 06BC E232 diff --git a/swig/python/setup.py b/swig/python/setup.py index b980b87..1edd509 100644 --- a/swig/python/setup.py +++ b/swig/python/setup.py @@ -12,13 +12,13 @@ from distutils.core import setup, Extension swig_time = os.path.getmtime('../jellyfish.i') older = True try: -older = os.path.getmtime('jellyfish_wrap.cxx') < swig_time or os.path.getmtime('jellyfish.py') < swig_time +older = os.path.getmtime('jellyfish_wrap.cxx') < swig_time or os.path.getmtime('dna_jellyfish.py') < swig_time except FileNotFoundError: older = True if older: -print("Running swig: swig -c++ -python -o jellyfish_wrap.cxx ../jellyfish.i") -os.system("swig -c++ -python -o jellyfish_wrap.cxx ../jellyfish.i") +print("Running swig: swig -c++ -python -module dna_jellyfish -o jellyfish_wrap.cxx ../jellyfish.i") +os.system("swig -c++ -python -module dna_jellyfish -o jellyfish_wrap.cxx ../jellyfish.i") jf_include = [re.sub(r'-I', '', x) for x in os.popen("pkg-config --cflags-only-I jellyfish-2.0").read().rstrip().split()] jf_cflags = os.popen("pkg-config --cflags-only-other jellyfish-2.0").read().rstrip().split() @@ -28,7 +28,7 @@ jf_libdir = [re.sub(r'-L', '', x) for x in os.popen("pkg-config --libs-only-L j jf_ldflags = os.popen("pkg-config --libs-only-other jellyfish-2.0").read().rstrip().split() -jellyfish_module = Extension('_jellyfish', +jellyfish_module = Extension('_dna_jellyfish', sources = ['jellyfish_wrap.cxx'], include_dirs = jf_include, libraries = jf_libs, @@ -36,9 +36,9 @@ jellyfish_module = Extension('_jellyfish', extra_compile_args = ["-std=c++0x"] + jf_cflags, extra_link_args = jf_ldflags,
Bug#819016: jellyfish: Rename python bindings module name
An update on the python tests running, not as exhaustive as I'd like due to having less time available than initially expected: upon closer inspection, I believe that indeed the python tests at swig/python/test*.py are not being currently run during the build, as hinted on the previous message. I have tried passing the location of the test folder to unittest using: export PYBUILD_TEST_ARGS=${CURDIR}/swig/python/ and the tests are picked up during pybuild, although they are erroring probably due to the library not being still available at that point: ImportError: libjellyfish-2.0.so.2: cannot open shared object file: No such file or directory -- Diego M. Rodriguez 36B3 42A9 9F2F 2CFB F79B FF9B B6C4 B901 06BC E232 signature.asc Description: Digital signature
Bug#819016: jellyfish: Rename python bindings module name
After some more testing, I'm wondering if it would be sensible to just *not* aim for having the python tests run during pybuild, and instead stick to running them on a separate stage (or during autopkgtest, which I have not ventured into yet). The main reason is that one of the tests (swig/python/test_mer_file.py) seems not really be meant to be executed using the standard unittest module, as it relies on a "data" variable created during __main__(), plus some hard-coded references to files generated during tests/swig_python.sh. I made some attempts to running the tests during pybuild by: * building the extension with rpath, removing it with chrpath --delete right afterwards (kind of negating the "drop-rpath" patch temporarily): export PYBUILD_BUILD_ARGS=build_ext --rpath "${CURDIR}/debian/tmp/usr/lib/${DEB_HOST_MULTIARCH}" ... pybuild ... chrpath --delete ... * patching the tests to provide a valid value for the "data" variable when run via unittest. * using PYBUILD_BEFORE_TEST and PYBUILD_AFTER_TEST to generate the files required by the test and place them in a reachable directory, cleaning up afterwards. While it seems doable (and probably prone to be refined), it feels rather unorthodox and introducing some extra complexity for a seamingly small gain - I'm still not that familiar with the package's build system and specifics, but suspect that there are better ways to tackle this issue, and I'd appreciate some hints or thoughts on the best course of action. Best regards, -- Diego M. Rodriguez 36B3 42A9 9F2F 2CFB F79B FF9B B6C4 B901 06BC E232 signature.asc Description: Digital signature
Bug#819016: jellyfish: Rename python bindings module name
Hi Diego, thanks for diving into this. On Fri, Mar 25, 2016 at 07:09:18PM +0100, Diego M. Rodriguez wrote: > After some more testing, I'm wondering if it would be sensible to just > *not* aim for having the python tests run during pybuild, and instead > stick to running them on a separate stage (or during autopkgtest, which > I have not ventured into yet). The main reason is that one of the tests > (swig/python/test_mer_file.py) seems not really be meant to be executed > using the standard unittest module, as it relies on a "data" variable > created during __main__(), plus some hard-coded references to files > generated during tests/swig_python.sh. I need to admit I'm in favour of running any test at build time as well as in an autopkgtest (see Debian Continuous Integration) as far as it is sensible. So if it turns out that parts of the test suite can not sensibly be run under every condition only this part should be excluded. > I made some attempts to running the tests during pybuild by: > * building the extension with rpath, removing it with chrpath --delete > right afterwards (kind of negating the "drop-rpath" patch temporarily): > export PYBUILD_BUILD_ARGS=build_ext --rpath > "${CURDIR}/debian/tmp/usr/lib/${DEB_HOST_MULTIARCH}" > ... > pybuild ... > chrpath --delete ... > * patching the tests to provide a valid value for the "data" variable > when run via unittest. > * using PYBUILD_BEFORE_TEST and PYBUILD_AFTER_TEST to generate the files > required by the test and place them in a reachable directory, cleaning > up afterwards. > > While it seems doable (and probably prone to be refined), it feels > rather unorthodox and introducing some extra complexity for a seamingly > small gain - I'm still not that familiar with the package's build system > and specifics, but suspect that there are better ways to tackle this > issue, and I'd appreciate some hints or thoughts on the best course of > action. I have not yet found time to dive into python-jellyfish yet so I'm just saying what I would try to approach: If it is easily possible to deactivate this part of the test suite that seems troublesome I would go for it. Kind regards Andreas. -- http://fam-tille.de
Bug#819016: jellyfish: Rename python bindings module name
Hello Andreas, > I need to admit I'm in favour of running any test at build time as well > as in an autopkgtest (see Debian Continuous Integration) as far as it is > sensible. So if it turns out that parts of the test suite can not > sensibly be run under every condition only this part should be excluded. I see your point, although I was actually not suggesting to skip the tests entirely at build time, but rather to move their execution from pybuild to another dh stage, with the reasoning being that "as long as the tests are executed, there is some leeway in deciding which tool invokes them". Perhaps running "pybuild ... --disable test/python3" within override_dh_auto_build, and then relying on the main makefile to actually perform tests/swig_python.sh during dh_auto_test would be an option? I haven't tried that approach yet, and some mangling would probably be needed to make dh_auto_test aware that the python extension is available and compiled, but might be doable and have the benefit that (hopefully) all the tests would be run. > I have not yet found time to dive into python-jellyfish yet so I'm just > saying what I would try to approach: If it is easily possible to > deactivate this part of the test suite that seems troublesome I would > go for it. I have tried using the "--pattern" argument to unittest discover [1] to exclude the problematic test_mer_file.py file, as in: export PYBUILD_TEST_ARGS=${CURDIR}/swig/python/test_string_mers.py -p "test_[!m]*.py" but unfortunately ran into another problem: it seems the test results depends on the order they are run: $ python3 -m unittest test_hash_counter.py test_string_mers.py ..F. == FAIL: test_all_mers (test_string_mers.TestStringMers) -- Traceback (most recent call last): File "/tmp/x/test_string_mers.py", line 21, in test_all_mers self.assertTrue(good) AssertionError: False is not true -- Ran 4 tests in 0.048s FAILED (failures=1) $ python3 -m unittest test_string_mers.py test_hash_counter.py -- Ran 4 tests in 0.049s OK I'm still not familiar with jellyfish, but I'm guessing that this is by design and the library shares some "global" state of sorts (which the current python test suite has been designed around)? To work around this issue, I'm including a patch that runs the tests individually using pybuild (and performs the rpath modification mentioned on #15 - its only purpose is making libjellyfish-2.0.so available during the tests, there might be a better way of achieving it). It should work, but I'm not really happy with the solution and would be more than willing to help trying to find a better approach if needed. Best regards, and thanks a lot for taking the time to look into the issue, [1] https://docs.python.org/3/library/unittest.html#cmdoption-unittest-discover-p -- Diego M. Rodriguez 36B3 42A9 9F2F 2CFB F79B FF9B B6C4 B901 06BC E232 diff --git a/debian/rules b/debian/rules index 5d9557e..3fbb357 100755 --- a/debian/rules +++ b/debian/rules @@ -13,6 +13,7 @@ export PKG_CONFIG_LIBDIR=${CURDIR} export PKG_CONFIG_ALLOW_SYSTEM_LIBS=true export PKG_CONFIG_SYSROOT_DIR=${CURDIR}/debian/tmp/ export PERL_MM_OPT=INSTALLDIRS=vendor +export PYBUILD_BUILD_ARGS=build_ext --rpath "${CURDIR}/debian/tmp/usr/lib/${DEB_HOST_MULTIARCH}" %: dh $@ --with autoreconf,python3 #--parallel @@ -23,7 +24,10 @@ override_dh_auto_install: override_dh_install: # dh_install -X*.a -X*.la -Xpkgconfig dh_install -ppython3-jellyfish - pybuild -d swig/python --name jellyfish + pybuild -d swig/python --name jellyfish --disable test/python3 + pybuild -d swig/python --name jellyfish --test --test-args "${CURDIR}/swig/python/ test_string_mers.py" + pybuild -d swig/python --name jellyfish --test --test-args "${CURDIR}/swig/python/ test_hash_counter.py" + chrpath --delete debian/python3-jellyfish/usr/lib/python*/dist-packages/_dna_jellyfish*.so dh_auto_configure --sourcedirectory=swig/perl5 dh_auto_build --sourcedirectory=swig/perl5 signature.asc Description: Digital signature
Bug#819016: jellyfish: Rename python bindings module name
Hello Andreas, following the latest discussion at #806716 [1], I'm attaching a patch that is intended to replace the "update-debian-rules-dh_clean.patch" on the first message of this bug report, and be used in conjunction with the "rename-python-module-to-dna_jellyfish.patch". Hopefully both of them accomplish the renaming of the python module (to "dna_jellyfish", as per the upstream's author light suggestion on the mailing list some time ago), and of the Debian binary package (to "python3-dna-jellyfish"). There is still the issue of running the Python tests to be solved (ie. they are currently not run in the unpatched version, and the patches above still don't change the situation). I have been trying to find a better solution than the one proposed on message #25, still with no luck. I'm wondering if you could provide input on whether the approach on #25 is acceptable, or some hints or ideas on how to better solve the test running issue? I'm also wondering if it would be a good time to try to contact upstream again in the hopes of eventually fixing the conflict upstream, maybe via github? I'd be willing to do so, provided you find it acceptable! Best regards, and thanks again, [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=806716#109 -- Diego M. Rodriguez 36B3 42A9 9F2F 2CFB F79B FF9B B6C4 B901 06BC E232 diff --git a/debian/control b/debian/control index 8f55e6a..8ab8176 100644 --- a/debian/control +++ b/debian/control @@ -90,7 +90,7 @@ Description: count k-mers in DNA sequences (development files of jellyfish) This package contains the development files (static library and header files) -Package: python3-jellyfish +Package: python3-dna-jellyfish Architecture: any-amd64 Section: python Depends: ${python3:Depends}, diff --git a/debian/rules b/debian/rules index 790ad39..7be0f55 100755 --- a/debian/rules +++ b/debian/rules @@ -22,8 +22,8 @@ override_dh_auto_install: override_dh_install: # dh_install -X*.a -X*.la -Xpkgconfig - dh_install -ppython3-jellyfish - pybuild -d swig/python --name jellyfish + dh_install -ppython3-dna-jellyfish + pybuild -d swig/python --name dna-jellyfish dh_auto_configure --sourcedirectory=swig/perl5 dh_auto_build --sourcedirectory=swig/perl5 @@ -55,7 +55,7 @@ override_dh_clean: rmdir debian/tmp_save_tests ; \ fi rm -f tests/compat.sh - rm -f swig/python/jellyfish* + rm -f swig/python/jellyfish* swig/python/dna_jellyfish.py override_dh_auto_build: mkdir -p debian/tmp_save_tests signature.asc Description: Digital signature
Bug#806716: Bug#819016: jellyfish: Rename python bindings module name
Hi Diego, at first thanks a lot for all your patches and specifically the patience you had since I was simply waiting for the next upstream version to take action on this issue. Since upstream has now released 2.2.6 I was updating the package, applied the patches and uploaded (but it will need to pass new queue due to the package name change). On Sat, Jul 09, 2016 at 06:53:00PM +0200, Diego M. Rodriguez wrote: > > There is still the issue of running the Python tests to be solved (ie. > they are currently not run in the unpatched version, and the patches > above still don't change the situation). I have been trying to find a > better solution than the one proposed on message #25, still with no > luck. I'm wondering if you could provide input on whether the approach > on #25 is acceptable, or some hints or ideas on how to better solve the > test running issue? Well, the test was passing in the new upstream version. May be the issue to run in the right sequence was solved? At least I have not noticed any problem. > I'm also wondering if it would be a good time to try to contact upstream > again in the hopes of eventually fixing the conflict upstream, maybe > via github? I'd be willing to do so, provided you find it acceptable! As I said I think its an acceptable solution. I have reported the issue upstream[1]. Kind regards Andreas. [1] https://github.com/gmarcais/Jellyfish/issues/69 -- http://fam-tille.de
Bug#806716: Bug#819016: jellyfish: Rename python bindings module name
Hello Andreas, > at first thanks a lot for all your patches and specifically the patience > you had since I was simply waiting for the next upstream version to take > action on this issue. Since upstream has now released 2.2.6 I was > updating the package, applied the patches and uploaded (but it will need > to pass new queue due to the package name change). I'm glad the patches were useful and that the new release is on its way, thanks a lot for your help and cooperation on solving the issue. I'll wait until the package passes the new queue and the dust settles to take further action on this package, but it's great to finally be close to being able to mark this issue completed. > Well, the test was passing in the new upstream version. May be the > issue to run in the right sequence was solved? At least I have not > noticed any problem. At a first glance, I couldn't pinpoint what exactly caused the test order issue to be solved, but it seems to be the case. In any case, if it reappears please ping me back and I'd be more than happy to lend a hand. > > I'm also wondering if it would be a good time to try to contact upstream > > again in the hopes of eventually fixing the conflict upstream, maybe > > via github? I'd be willing to do so, provided you find it acceptable! > > As I said I think its an acceptable solution. I have reported the issue > upstream[1]. Thanks as well for doing so! I'll be keeping an eye on the github issue, in the hopes of reaching a more "universal" solution. Again, thanks a lot, Andreas! -- Diego M. Rodriguez 36B3 42A9 9F2F 2CFB F79B FF9B B6C4 B901 06BC E232 signature.asc Description: Digital signature