python3Packages.dendropy: 4.5.1 -> 5.0.2 #326199
Conversation
Pandapip1
added
0.kind: package adoption
|
Result of 4 packages built:
|
Pandapip1
force-pushed
the
dendropy-update
branch
3 times, most recently
from
9ba2a70
to
982f067
Compare
Pandapip1
force-pushed
the
dendropy-update
branch
2 times, most recently
from
5c4347c
to
2bd2d34
Compare
|
Result of 5 packages built:
|
Pandapip1
force-pushed
the
dendropy-update
branch
from
2bd2d34
to
167d42c
Compare
|
|
||
| pythonImportsCheck = [ "dendropy" ]; | ||
|
|
||
| meta = with lib; { | ||
| env.DENDROPY_PAUP_EXECUTABLE_PATH = if paupIntegration then lib.getExe paup else "NONE"; |
There was a problem hiding this comment.
It's a bit tricky.
It seems reasonable to leave it to upstream to make sure the paup integration works, and we disable it.
There was a problem hiding this comment.
Annoyingly, upstream doesn't make sure the paup integration works. If you remove this line, you'll get failed tests because it can't find the paup binary.
There was a problem hiding this comment.
In any case, paup is marked as unfree, so our CI will not run the test either.
There was a problem hiding this comment.
In any case, paup is marked as unfree, so our CI will not run the test either.
There are four tests that test PAUP integration. They are disabled when DENDROPY_PAUP_EXECUTABLE_PATH is set to NONE. They are not disabled when the DENDROPY_PAUP_EXECUTABLE_PATH environment variable is not set. Then, dendropy just assumes that PAUP is in the PATH.
Specifically, those tests are:
"test_basic_split_count_with_incorrect_rootings_raises_error"
"test_basic_split_count_with_incorrect_weight_treatment_raises_error"
"test_basic_split_counting_under_different_rootings"
"test_group1"
These tests used to be disabled. However, I changed this so that those tests can be run if PAUP is enabled.
Also worth noting: the author of PAUP has plans to make it open-source later this year.
There was a problem hiding this comment.
The upstream looks like it is running an integration test.
https://github.com/jeetsukumaran/DendroPy/blob/a27a6d46e5a7e1c9573b3022a18f3005cd8b996e/.github/workflows/ci.yaml#L30-L37
I'm not willing to expose checkValidity just for the four tests in this package.
There was a problem hiding this comment.
You're right in regards of just four unit tests being insufficient to expose checkValidity. I meant to additionally figure out a way to make PAUP* integration work for dependents of dendropy.
Also, there are a couple other packages where access to checkValidity is useful (#310138 integration with monado, for example). In fact, any open source project with an optional non-free dependency could really make use of checkValidity.
There was a problem hiding this comment.
I'm not very familiar with check-meta, so I can't decide if it should be exposed.
However, I agree that there are a number of useful examples.
How about implementing this suggestion in another PR?
I think it would get more people involved in the discussion.
There was a problem hiding this comment.
Okay, then I will make it a separate PR.
Pandapip1
force-pushed
the
dendropy-update
branch
from
167d42c
to
8a2e721
Compare
Pandapip1
force-pushed
the
dendropy-update
branch
2 times, most recently
from
67a65c8
to
6a3575c
Compare
Pandapip1
changed the title
|
Updated to 5.0.2 Would really like this to be merged. It's blocking #311463 |
|
Result of 5 packages built:
|
Pandapip1
force-pushed
the
dendropy-update
branch
from
6a3575c
to
c53bb22
Compare
|
Had some nixfmt errors. Those are now fixed. |
Pandapip1
force-pushed
the
dendropy-update
branch
from
c53bb22
to
6d1c50c
Compare
Pandapip1
force-pushed
the
dendropy-update
branch
from
6d1c50c
to
0c28817
Compare
|
Result of 5 packages built:
|
|
Not backporting because this is a major version bump. |
Description of changes
Updates
dendropyto the latest version, revamps the derivation, and adds myself as maintainer.Please see my comments for explanation about some of the changes I made.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.