Bug 2246777 - Review Request: python-chart-studio - Utilities for interfacing with plotly's Chart Studio
Summary: Review Request: python-chart-studio - Utilities for interfacing with plotly's...
Keywords:
Status: CLOSED DEFERRED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: fedora-neuro, NeuroFedora
TreeView+ depends on / blocked
 
Reported: 2023-10-28 20:18 UTC by Sandro
Modified: 2023-11-06 14:00 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2023-11-06 14:00:33 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Sandro 2023-10-28 20:18:04 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/gui1ty/reviews/fedora-rawhide-x86_64/06576318-python-chart-studio/python-chart-studio.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/gui1ty/reviews/fedora-rawhide-x86_64/06576318-python-chart-studio/python-chart-studio-1.1.0-5.src.rpm

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

Comment 1 Fedora Review Service 2023-10-29 01:33:33 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6577364
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2246777-python-chart-studio/srpm-builds/06577364/builder-live.log.gz

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.

Comment 2 Sandro 2023-10-29 09:04:47 UTC
[fedora-review-service-build]

Should this fail again, there is a Copr build: https://copr.fedorainfracloud.org/coprs/gui1ty/reviews/build/6576318/

Comment 3 Ben Beasley 2023-10-29 13:00:48 UTC
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.

Comment 4 Sandro 2023-10-29 13:38:59 UTC
(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/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.

Comment 5 Ben Beasley 2023-10-29 13:57:43 UTC
(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/jupyterlab-plotly. 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.

Comment 6 Sandro 2023-10-29 14:18:24 UTC
(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

Comment 7 Ben Beasley 2023-10-29 14:26:51 UTC
(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/45578ffeb0905a0b3d78ac1cc7ed99a16fcf1795/f/python-pybv.spec#_14.

Comment 8 Sandro 2023-10-29 15:00:28 UTC
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. 🤨

Comment 9 Ben Beasley 2023-10-29 15:14:36 UTC
(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_licenses

Comment 10 Sandro 2023-11-06 14:00:33 UTC
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/neurofedora-2023-11-06.2023-11-06-13.02.html


Note You need to log in before you can comment on or make changes to this bug.