networkx fails to build with SAGE_CHECK="yes"

[Konrad127123]

After #26326, networkx fails to build with SAGE_CHECK="yes" unless the optional package nose is installed.

[networkx-2.2] Running the test suite for networkx-2.2...
[networkx-2.2] Testing networkx requires the package nose to be installed
[networkx-2.2]
[networkx-2.2] real     0m0.004s
[networkx-2.2] user     0m0.004s
[networkx-2.2] sys      0m0.001s
[networkx-2.2] ************************************************************************
[networkx-2.2] Error testing package networkx-2.2
[networkx-2.2] ************************************************************************

Component: packages: standard

Keywords: networkx, nose

Author: Konrad K. Dabrowski

Branch: e1ce380

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/27253

[Konrad127123]

Branch: u/Konrad127123/networkx_fails_to_build_with_sage_check__yes_

[Konrad127123]

comment:2

Suggested fix: add nose as a runtime dependency of networkx. This means nose needs to be a standard package.

---

New commits:

e1ce380

networkx needs a runtime dependency on nose, otherwise it fails to build with SAGE_CHECK="yes". This means nose has to be upgraded to a standard package.

[Konrad127123]

Author: Konrad K. Dabrowski

[Konrad127123]

Commit: e1ce380

[dimpase]

comment:3

Can you dig up the info on nose's license. Cause it needs to be GPL-compatible to be standard.

[dimpase]

comment:4

OK, it's fine. See https://github.com/nose-devs/nose/blob/master/lgpl.txt
Could you please update the nose's SPKG.txt file with this info?

[Konrad127123]

comment:5

nose's SPKG.txt already contains

== License ==

GNU LGPL

[dimpase]

Reviewer: Dima Pasechnik

[dimpase]

comment:6

OK, sorry, I was obviously blind :-)

[vbraun]

Changed branch from u/Konrad127123/networkx_fails_to_build_with_sage_check__yes_ to e1ce380

[vbraun]

Changed commit from e1ce380 to none

[vbraun]

comment:8

Guys, test harnesses are not automatically standard packages. Instead the spkg-check script should just abort if nose is not found.

[jhpalmieri]

comment:9

So is nose going to be standard (bypassing the required discussion and vote) or is this change going to be reverted?

[dimpase]

comment:10

I think this falls under a situation of a new requirement by a standard package.
We have been adding various Python deps like this quite easily...

[jhpalmieri]

comment:11

I view Volker's comment as disagreeing with you. Could you respond to that, please?

[dimpase]

comment:12

Building a standard package with SAGE_CHECK=yes must work. If anyone can offer a different solution to this, without making nose standard, fine...

But please do not offer removing checks just cause you do not want to promote nose...

[jhpalmieri]

comment:13

Replying to @dimpase:

Building a standard package with SAGE_CHECK=yes must work. If anyone can offer a different solution to this, without making nose standard, fine...

Volker did that.

But please do not offer removing checks just cause you do not want to promote nose...

But you don't like his solution, so you're discarding it.

The use of nose was added at #26326, and that ticket description says quite clearly, "Note that self tests require nose which is optional only." So it's not like this situation is a surprise or was unintended when the self-tests were implemented. I would deduce that the intention was: if nose is present, use it, otherwise skip the self tests. Or maybe the intention was: most people don't run the self tests, and those that do can install nose. In any case, I don't agree that #26326 forces everyone to install nose.

[dimpase]

comment:14

A Dieselgate-style "solution" is not a solution. This is already merged, and let us not start track ticket wars caused by a 300K addition to Sage tarballs.

[embray]

comment:15

ISTM running the tests for networkx also requires scipy, so perhaps scipy should be a prerequisite dependency.

[dimpase]

comment:16

Replying to @embray:

ISTM running the tests for networkx also requires scipy, so perhaps scipy should be a prerequisite dependency.

yes, this is a good catch. Should we do a quick gitlab fix?

[embray]

comment:17

Followup in #27515