Repository navigation
Sourceless files astropy/table/data/jquery*.js; without explicite license #1992
Description
This is a forward of the Debian bug report 735770:
The files
astropy/table/data/jquery-1.10.2.min.jsastropy/table/data/jquery.dataTables.min.js
are not really source files since they are somehow uglified and thus not editable in the form provided by Astropy. Also, they are not mentioned in the licenses/ subdirectory.
Are these files needed at all? They are introduced with the comment
minify js
which does not really enlighten me. And, they are 3rd party code (as seen from the headers), without being in the external/ subdirectory. For Debian packaging, I would like to avoid the use of them.
Metadata
Metadata
Assignees
Labels
No labels
Activity
astrofrog commentedon Jan 23, 2014
cc @keflavich
These are the libraries from http://jquery.com/ - I agree they should be in
external.mdboom commentedon Jan 23, 2014
Note that IPython uses/includes jquery as well. But it includes with the source and minified versions in the IPython source tree. I think we have to do that as well, because there aren't any good ways to minify Javascript from our build script without adding another most likely non-Python-based dependency.
The Debian package of IPython may offer some guidance. There, the Debian package appears to depend on
libjs-jqueryand then IPython's paths are updated accordingly to use it instead of the built-in ones.olebole commentedon Jan 23, 2014
@mdboom This is what I would do as well. But, there is a source check before, and I must ensure that the original tarball does not include files without source; so I need to modify the source tar file. Optimal for me would be to have an additional "--use-external" flag which would avoid copying these files to the installation.
However, I still did not understand yet: what are these files for in the astropy context? How are they to be used?
What concerns the current 0.3 release: what would I loose if I just remove them (to fix the failure which is considered as [Debian] Release Critical bug and must be fixed ASAP)?
mdboom commentedon Jan 23, 2014
When in an IPython notebook and you display a
astropy.Tableobject, the table is displayed as an HTML table with some sorting magic and other UI nicities. This is all based on jquery. If you remove them, that functionality will not work (and will probably result in some sort of file-not-found error).keflavich commentedon Jan 24, 2014
This is a relatively minor (and probably not well-known) feature at present, and I think the file-not-found error would likely result in a reasonably formatted HTML table without any javascript bells & whistles; from my vague memories, that's what happened when I was debugging.
@olebole - apologies for the uninformative commit message; the full commit log from this PR: #1342 is probably a good deal more helpful
olebole commentedon Jan 24, 2014
Thank you, so I am going to remove these files from the Debian version 0.3 to fix the bug there.
mdboom commentedon Jan 24, 2014
Just confirmed that the failure here is a table without any of the bells or whistles (but still quite viewable).
In the end, we should probably fix this to use the system-installed jquery from Debian if available, or we'll eventually get a bug report that the Debian package is broken. 😉
Alternatively, I wonder if switching these links to the CDN of these files wouldn't be just as good. We can get them out of our source tree. It does mean that the user will need an internet connection the first time they use this feature, but not subsequently. There's also a very good change their browser already has this file in their cache.
I personally like the second option because it means Debian doesn't have to do anything special with our package, and the behavior is the same when built from source as when installed from
apt-get.keflavich commentedon Jan 24, 2014
I still like to keep the option of using a js viewer after I clear my
browser cache while on an airplane... but I understand that's really a
corner case and should not come up often, so switching back to links makes
sense.
olebole commentedon Jan 24, 2014
I find it OK if the *.js files are in the tar file, as long as they come with the "real" (human-readable) source, and if there is an option to use system provided ones by applying an option to the install script. Preferrably, these files should be moved to
external/, and mentioned in thelicenses/subdir.jquery.js is installed as
/usr/share/javascript/jquery/jquery.min.jsinlibjs-jquery, as already mentioned my @mdboom. Is there a way to use this installed version if available?Unfortunately,
jquery.dataTable.jsis not packaged yet for Debian, but someone intents to package it. Once it is available, we could have this functionality as well on Debian. I will ask there about the status of the packaging.embray commentedon Jan 27, 2014
I think just a couple easy to tweak global variables should contain the paths to these files; the Debian package can easily be updated to point to the system copy if need be, like is done for IPython.
eteq commentedon Jan 28, 2014
This feature is intended entirely for use with the IPython notebook, right? If so, perhaps jquery doesn't need to be in astropy at all, if it's already in IPython? (Or is it that
dataTableis not in IPython?)olebole commentedon Jan 28, 2014
@embray I agree here; however it is important for me that these (minified) files are not in the tar file as long as they do not come with source (what means the "original", easily editable one). The major problem I have in the moment is that I need to repack the astropy-0.3 sources in order to clean out the
jquery*.jsfiles.@eteq If it is basically a IPython feature, I would propose not to include it in astropy at all, since (I think) the goal of astropy is not to unspecifically extend IPython's UI features.
mdboom commentedon Jan 28, 2014
This feature is usable outside of IPython, which in any event doesn't include or depend on jquery.dataTable, so that unfortunately doesn't really help us.
Using the remote CDN seems like the easiest road out, with the IMHO corner case that it won't work the first time without a network connection.
@olebole: Would just including the non-minified source alongside these files resolve the issue, as far as Debian is concerned?
olebole commentedon Jan 28, 2014
@mdboom This helps to to avoid the "serious" (RC) bug that went to this issue. However, having minified js files in the tar file rings a bell in Debian as well. If this feature is really needed here, would be an option to run a JS minifier during astropy's build?
What concerns the remote CDN, this may lead to privacy issues, since it allows to trace the (first) use of this feature without the explicite possibility of an opt-out. It is something I would disable in the Debian package.
Sorry that Debian is a bit picky here -- in any case I do not force that astropy follows its rules; there are almost always ways for me to work around them (repackaging tarball, patching sources); so let's see to get a good compromise here.
eteq commentedon Jan 28, 2014
@olebole - Would either of these alternatives work?:
extern).embray commentedon Jan 29, 2014
A third option is to just not include the minified versions at all, and just include the full source. The uncompressed jquery is ~250kB and gzipped reduces to about 80 kB. For the purpose of packaging with Astropy there is nothing significant otherwise gained by including the minified versions (much less only the minified versions).
A fourth would be to take the CDN route, but if we do that the JS will need to be updated to give some useful and easy to find error messages if the feature does not work. I'm not comfortable shipping a feature that silently breaks if the user doesn't have internet access.
olebole commentedon Jan 29, 2014
@eteq a clear notice would be good in any case (as for other external sources); although I provide an alternative
debian/copyrightfile which collects all copyrights and which is maintained independently. However, the requirement is that the package includes all sources, so if there is a minified version, also the original one should be there. And, I must build all files that are used in the Debian package from their sources, so if I would go to use the minified version, I would need to build them from the source myself.For the CDN approach, an opt-out would be good; however I would reverse this to an opt-in.
My preference for the Debian package would be:
jquery.jsandjquery.dataTable.jsprovided by Astropylibjs-jquerylibrary to providejquery.jsfunctionalitylibjs-jquery-datatablepackage which would providejquery.dataTable.jsfunctionality once it is packaged. If it turnes out that packaginglibjs-jquery-datatabletakes longer, I would provide this file for the time being, however I would need to compile it from the (unminified) source then.mdboom commentedon Jan 29, 2014
I'm going to work up a PR with some of these ideas.
astrofrog commentedon May 9, 2014
Fixed by #2020