Opened 2 years ago
Closed 23 months ago
#25106 closed defect (fixed)
Don't require matplotlib or sympy to run doctests
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.2 |
Component: | doctest framework | Keywords: | |
Cc: | SimonKing | Merged in: | |
Authors: | Jeroen Demeyer | Reviewers: | Julian Rüth |
Report Upstream: | N/A | Work issues: | |
Branch: | 058ae5f (Commits) | Commit: | 058ae5fae44b940084063dc852cd304627c1db25 |
Dependencies: | Stopgaps: |
Description (last modified by )
The doctest framework itself should not depend on matplotlib or sympy. This is because some other packages (p_group_cohomology
for example) use the Sage doctest framework but do not otherwise require those packages.
Change History (14)
comment:1 Changed 2 years ago by
- Description modified (diff)
comment:2 Changed 2 years ago by
- Branch set to u/jdemeyer/don_t_require_matplotlib_to_run_doctests
comment:3 Changed 2 years ago by
- Commit set to 058ae5fae44b940084063dc852cd304627c1db25
comment:4 Changed 2 years ago by
- Description modified (diff)
- Status changed from new to needs_review
- Summary changed from Don't require matplotlib to run doctests to Don't require matplotlib or sympy to run doctests
comment:5 follow-up: ↓ 6 Changed 2 years ago by
- Cc SimonKing added
This looks okay to me. Simon, any opinions?
comment:6 in reply to: ↑ 5 Changed 2 years ago by
Replying to jhpalmieri:
This looks okay to me. Simon, any opinions?
I'd find it very good if one could use the SageMath doctest framework with as few prerequisites as possible. So, if this branch makes it so that sympy and matplotlib are not required to run doctests, I am +1!
comment:7 follow-up: ↓ 8 Changed 2 years ago by
On the other hand: Both sympy and matplotlib are standard packages, and I don't see what harm would be done if Sage requires two of its standard packages to run tests.
comment:8 in reply to: ↑ 7 ; follow-up: ↓ 9 Changed 2 years ago by
comment:9 in reply to: ↑ 8 Changed 2 years ago by
Replying to jdemeyer:
Replying to SimonKing:
I don't see what harm would be done if Sage requires two of its standard packages to run tests.
There is harm if the Sage doctesting framework requires two of Sage's standard packages to run tests because the Sage doctesting framework is used for non-Sage tests too in #18514.
No it isn't. The p_group_cohomology
spkg has always and foremost been a Sage package (that's what spkg stands for). So, the p_group_cohomology
's test suite requires Sage being present. Otherwise it would simply not run.
Best regards, Simon
comment:10 Changed 2 years ago by
This ticket is not about p_group_cohomology. I don't think that the Sage doctesting framework should require all standard Sage packages.
comment:11 Changed 23 months ago by
I'm willing to give this a positive review. I don't see a downside to it, and there are potential advantages, even if there are no immediate uses for it yet (#18514 doesn't require it, I think).
comment:12 follow-up: ↓ 13 Changed 23 months ago by
- Reviewers set to Julian Rüth
- Status changed from needs_review to positive_review
Looks good to me. jhpalmieri, feel free to add yourself to the reviewer list.
jdemeyer: So, our doctesting framework could/should be an independent standard SPKG then?
comment:13 in reply to: ↑ 12 Changed 23 months ago by
Replying to saraedum:
jdemeyer: So, our doctesting framework could/should be an independent standard SPKG then?
Depends what you mean with "independent". It should certainly be usable by other packages besides Sage (for example p_group_cohomology
). But it still depends on pieces of Sage, so it's not that simple to make it truly independent.
comment:14 Changed 23 months ago by
- Branch changed from u/jdemeyer/don_t_require_matplotlib_to_run_doctests to 058ae5fae44b940084063dc852cd304627c1db25
- Resolution set to fixed
- Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
Don't require matplotlib or sympy to run doctests