[Bug 2246777] New: Review Request: python-chart-studio - Utilities for interfacing with plotly's Chart Studio
https://bugzilla.redhat.com/show_bug.cgi?id=2246777
Bug ID: 2246777 Summary: Review Request: python-chart-studio - Utilities for interfacing with plotly's Chart Studio Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: gui1ty@penguinpee.nl QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://download.copr.fedorainfracloud.org/results/gui1ty/reviews/fedora-raw... SRPM URL: https://download.copr.fedorainfracloud.org/results/gui1ty/reviews/fedora-raw...
Description: This package contains utilities for interfacing with Plotly's Chart Studio service (both Chart Studio cloud and Chart Studio On-Prem). Prior to plotly.py version 4, This functionality was included in the plotly package under the plotly.plotly module. As part of plotly.py version 4, the Chart Studio functionality was removed from the plotly package and released in this chart-studio package.
Fedora Account System Username: gui1ty
https://bugzilla.redhat.com/show_bug.cgi?id=2246777
Sandro gui1ty@penguinpee.nl changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |1276941 | |(fedora-neuro,NeuroFedora) Doc Type|--- |If docs needed, set a value
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1276941 [Bug 1276941] Fedora NeuroImaging and NeuroScience tracking bug
https://bugzilla.redhat.com/show_bug.cgi?id=2246777
--- Comment #1 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/6577364 (failed)
Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please make sure the package builds successfully at least for Fedora Rawhide.
- If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2246777
--- Comment #2 from Sandro gui1ty@penguinpee.nl --- [fedora-review-service-build]
Should this fail again, there is a Copr build: https://copr.fedorainfracloud.org/coprs/gui1ty/reviews/build/6576318/
https://bugzilla.redhat.com/show_bug.cgi?id=2246777
Ben Beasley code@musicinmybrain.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |code@musicinmybrain.net
--- Comment #3 from Ben Beasley code@musicinmybrain.net --- At a glance:
- Everything that remains after %prep is indeed MIT, as advertised. Everything in the original source archive appears to have a license that is allowable in Fedora, but:
- The full license status of packages/python/plotly/plotly/package_data/plotly.min.js can’t be properly reviewed since it bundles and minifies dependencies without preserving their identities or license information.
- There is a typo, %pypproject_check_import
I know this has implications for python-plotly, but I am thinking we might not even be able to distribute plotly.min.js in source RPMs, let alone in binary RPMs, since it discards all the license information for its bundled dependencies, including mandatory license texts (e.g. for MIT and BSD family licenses). It contains a comment /*! For license information please see plotly.min.js.LICENSE.txt */, but no such file exists in the distribution.
https://bugzilla.redhat.com/show_bug.cgi?id=2246777
--- Comment #4 from Sandro gui1ty@penguinpee.nl --- (In reply to Ben Beasley from comment #3)
At a glance:
- Everything that remains after %prep is indeed MIT, as advertised.
Everything in the original source archive appears to have a license that is allowable in Fedora, but:
- The full license status of
packages/python/plotly/plotly/package_data/plotly.min.js can’t be properly reviewed since it bundles and minifies dependencies without preserving their identities or license information.
That's an issue to be addressed with regards to `python-plotly`. This package doesn't ship any Javascript files. Although, it's rather useless without `plotly`, should that no longer be permissible.
- There is a typo, %pypproject_check_import
Well spotted! Good thing it's not being used or the build would fail. ;) Easy fix!
I know this has implications for python-plotly, but I am thinking we might not even be able to distribute plotly.min.js in source RPMs, let alone in binary RPMs, since it discards all the license information for its bundled dependencies, including mandatory license texts (e.g. for MIT and BSD family licenses). It contains a comment /*! For license information please see plotly.min.js.LICENSE.txt */, but no such file exists in the distribution.
The PyPI sdist tarball - https://files.pythonhosted.org/packages/0d/17/ba496e60f95020227a15f73965a64e... - does contain several license files:
plotly-5.18.0/jupyterlab_plotly/nbextension/index.js.LICENSE.txt plotly-5.18.0/jupyterlab_plotly/labextension/static/third-party-licenses.json plotly-5.18.0/jupyterlab_plotly/labextension/static/486.6450efe6168c2f8caddb.js.LICENSE.txt plotly-5.18.0/jupyterlab_plotly/labextension/static/478.b48f45da3d88616ad3f9.js.LICENSE.txt plotly-5.18.0/LICENSE.txt
Would these be sufficient to clarify the applicable Javascript licenses? If so, I can have them included in the `python-plotly` package and update the License: tag accordingly, if needed.
https://bugzilla.redhat.com/show_bug.cgi?id=2246777
--- Comment #5 from Ben Beasley code@musicinmybrain.net --- (In reply to Sandro from comment #4)
That's an issue to be addressed with regards to `python-plotly`. This package doesn't ship any Javascript files. Although, it's rather useless without `plotly`, should that no longer be permissible.
It ships them in the source RPM. If something in the source archive violates packaging guidelines (like a precompiled executable), it’s enough to remove it in %prep. If there’s something that means we can’t redistribute a file from the source archive, we need to remove it before uploading to the lookaside cache so it doesn’t appear either in the lookaside cache or in the .src.rpm packages.
The PyPI sdist tarball - https://files.pythonhosted.org/packages/0d/17/ ba496e60f95020227a15f73965a64ea3f176cae7faed2d9302a14524b681/plotly-5.18.0. tar.gz - does contain several license files:
plotly-5.18.0/jupyterlab_plotly/nbextension/index.js.LICENSE.txt plotly-5.18.0/jupyterlab_plotly/labextension/static/third-party-licenses.json plotly-5.18.0/jupyterlab_plotly/labextension/static/486.6450efe6168c2f8caddb. js.LICENSE.txt plotly-5.18.0/jupyterlab_plotly/labextension/static/478.b48f45da3d88616ad3f9. js.LICENSE.txt plotly-5.18.0/LICENSE.txt
Would these be sufficient to clarify the applicable Javascript licenses? If so, I can have them included in the `python-plotly` package and update the License: tag accordingly, if needed.
The top-level LICENSE.txt is just the “overall” MIT license; it doesn’t have anything about the JavaScript dependencies.
The others seem to pertain to the “Jupyter Extension for Plotly.py,” jupyterlab-plotly, https://github.com/plotly/plotly.py/tree/master/packages/javascript/jupyterl.... They do have some information about bundled dependencies and their licenses, but they lack the required license texts, and I’m not sure if they are complete. In any case, they don’t cover the massive dependency tree of https://github.com/plotly/plotly.js/, which is where the plotly.min.js bundle seems to come from.
https://bugzilla.redhat.com/show_bug.cgi?id=2246777
--- Comment #6 from Sandro gui1ty@penguinpee.nl --- (In reply to Ben Beasley from comment #5)
(In reply to Sandro from comment #4)
That's an issue to be addressed with regards to `python-plotly`. This package doesn't ship any Javascript files. Although, it's rather useless without `plotly`, should that no longer be permissible.
It ships them in the source RPM. If something in the source archive violates packaging guidelines (like a precompiled executable), it’s enough to remove it in %prep. If there’s something that means we can’t redistribute a file from the source archive, we need to remove it before uploading to the lookaside cache so it doesn’t appear either in the lookaside cache or in the .src.rpm packages.
Right. My bad. I forgot to take the SRPM and the tarballs in it into account.
In any case, they don’t cover the massive dependency tree of https://github.com/plotly/plotly.js/, which is where the plotly.min.js bundle seems to come from.
There is some information in the repository and on the website regarding building and bundling from scratch ([1],[2],[3]). So, to make `python-plotly` compliant one would need to build `plotly.js` from scratch and package it for Fedora. This package (`python-chart-studio`) and `python-plotly-geo` could then be build from the PyPI source tarball, which do not contain any Javascript files.
[1] https://github.com/plotly/plotly.js/blob/master/BUILDING.md [2] https://github.com/plotly/plotly.js/blob/master/CUSTOM_BUNDLE.md [3] https://github.com/plotly/plotly.js/blob/master/dist/README.md
https://bugzilla.redhat.com/show_bug.cgi?id=2246777
--- Comment #7 from Ben Beasley code@musicinmybrain.net --- (In reply to Sandro from comment #6)
There is some information in the repository and on the website regarding building and bundling from scratch ([1],[2],[3]). So, to make `python-plotly` compliant one would need to build `plotly.js` from scratch and package it for Fedora. This package (`python-chart-studio`) and `python-plotly-geo` could then be build from the PyPI source tarball, which do not contain any Javascript files.
That sounds about right.
For this package in isolation, if you wanted to use the GitHub archive in order get the tests, you could also add a comment above the Source (or add a script as an additional source) corresponding to the steps in %prep, then upload the result to dist-git for use as Source0.
See, for example, https://src.fedoraproject.org/rpms/python-pybv/blob/45578ffeb0905a0b3d78ac1c....
https://bugzilla.redhat.com/show_bug.cgi?id=2246777
--- Comment #8 from Sandro gui1ty@penguinpee.nl --- Let's see what the future holds for `plotly` before embarking on the endeavor of creating a custom source tarball.
Looking at the SRPM of `python-pybv` raises the question of what the license of `get_source` is. 🤨
https://bugzilla.redhat.com/show_bug.cgi?id=2246777
--- Comment #9 from Ben Beasley code@musicinmybrain.net --- (In reply to Sandro from comment #8)
Looking at the SRPM of `python-pybv` raises the question of what the license of `get_source` is. 🤨
I guess it could be made explicit, but like the spec file itself, it’s MIT-licensed by default under the FPCA[1]. This is mentioned for spec files in [2].
[1] https://docs.fedoraproject.org/en-US/legal/fpca/
[2] https://docs.fedoraproject.org/en-US/legal/license-approval/#_allowed_licens...
https://bugzilla.redhat.com/show_bug.cgi?id=2246777
Sandro gui1ty@penguinpee.nl changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Resolution|--- |DEFERRED Last Closed| |2023-11-06 14:00:33
--- Comment #10 from Sandro gui1ty@penguinpee.nl --- Since `plotly` might get dropped due to licensing issues wrt bundled NodeJS, this package might no longer be relevant --> closing.
Discussed in neuro-sig meeting: https://meetbot.fedoraproject.org/neuro_matrix_fedoraproject-org/2023-11-06/...
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com