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
