Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

split out a separate Python package primecountpy #32894

Closed
dimpase opened this issue Nov 18, 2021 · 62 comments
Closed

split out a separate Python package primecountpy #32894

dimpase opened this issue Nov 18, 2021 · 62 comments

Comments

@dimpase
Copy link
Member

dimpase commented Nov 18, 2021

This is a followup to #25009, and also resolves the bugs mentioned in #24960.

Here is my attempt (WIP) to create such a package:
https://github.com/dimpase/primecountpy

On PyPI: https://pypi.org/project/primecountpy/

Depends on #25009

CC: @mkoeppe

Component: packages: standard

Author: Dima Pasechnik

Branch/Commit: 74b3845

Reviewer: Matthias Koeppe

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

@dimpase dimpase added this to the sage-9.5 milestone Nov 18, 2021
@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 18, 2021

comment:2

As you know, https://pypi.org/project/primecount/ is already taken...

@dimpase
Copy link
Member Author

dimpase commented Nov 18, 2021

comment:3

I must say I don't quite know what exactly pypi package name is, and whether anything beyond changes in MANIFEST.in and setup.py is needed.
Can the module name still be the same, primecount ?

We'd call it pyprimecount instead.

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 18, 2021

comment:4

I explained it recently in https://groups.google.com/g/sage-devel/c/p4MO6L95s7E/m/yZF93ewIBAAJ, take a look

@dimpase
Copy link
Member Author

dimpase commented Nov 18, 2021

comment:5

OK, thanks. Another thing I don't understand is how the C++ library prinecount is meant to be provided. It's not something too standard to assume it's just installed.

Does it mean to be a non-python dependency of Sage? Or of (py)prinecount?

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 18, 2021

comment:6

When pyprimecount is installed from a source distribution, it will just assume that the necessary library is available. Python packages do not have a way to declare non-Python dependencies.

Later one may consider to build wheels that include a copy of the shared library.
This is what for example numpy does regarding BLAS: The source distribution certainly does not contain a copy of BLAS, but the wheels ship with a copy of the OpenBLAS shared library.

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 18, 2021

comment:7

On the Sage side, there will be two SPKGs: One for primecount, one for pyprimecount. This mirrors what is done, for example, for ppl and pplpy. The SPKG dependencies of pyprimecount include primecount; so when pyprimecount is pip-installed, the library has already been installed.

@dimpase

This comment has been minimized.

@dimpase
Copy link
Member Author

dimpase commented Nov 19, 2021

comment:8

after a very careful consideration, decided to rename it primecountpy.

@mkoeppe mkoeppe changed the title split out a separate Python package primecount split out a separate Python package primecountpy Nov 20, 2021
@dimpase
Copy link
Member Author

dimpase commented Nov 20, 2021

comment:10

OK, so prinecountpy works (needs setting up proper tests, etc), but other than that, what else is needed on that side?Some pypi/pip magic?

(on Sage's side clearly code to be removed and replaced by hopefully clear something...)

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 20, 2021

comment:11

when primecountpy is ready, use setup.py sdist to create an sdist and upload to PyPI with twine

Then on the Sage side, use ./sage -package create primecountpy --pypi

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 24, 2021

Dependencies: #25009

@dimpase

This comment has been minimized.

@dimpase
Copy link
Member Author

dimpase commented Nov 25, 2021

comment:13

OK, here it is: https://pypi.org/project/primecountpy/

@dimpase
Copy link
Member Author

dimpase commented Nov 25, 2021

Last 10 new commits:

c655136remove sagemath-primecount declaration
9ca1f67misc config changes for primesieve
d8e2cdaadjust deps,remove stuff from cfg and toml
3f1e441no need for CMAKE_INSTALL_PREFIX (it's in sdh_cmake)
abce309flyby patch: remove CMAKE_INSTALL_PREFIX, as it's set in sdh_cmake
a6de7f4check external primesieve with cmake
733cebcprime_pi and legendre_phi calling primecount
4f77073allow for slightly different Overflow messages in doctest
f7a0eedspkg primecountpy
9e154aduse primecountpy instead of built-in cython interface

@dimpase
Copy link
Member Author

dimpase commented Nov 25, 2021

Branch: u/dimpase/packages/primecountpy

@dimpase
Copy link
Member Author

dimpase commented Nov 25, 2021

Author: Dima Pasechnik

@dimpase
Copy link
Member Author

dimpase commented Nov 25, 2021

Commit: 9e154ad

@dimpase
Copy link
Member Author

dimpase commented Nov 25, 2021

comment:15

it works, apparently.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 25, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

f3d12bcremove primecount from various lists of optional spkgs

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 25, 2021

Changed commit from 9e154ad to f3d12bc

@dimpase

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 26, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

d950913fedora 30 doesn't have primecount pkg

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 26, 2021

Changed commit from f3d12bc to d950913

@mkoeppe
Copy link
Contributor

mkoeppe commented Nov 26, 2021

comment:19

this should go to #25009

@isuruf
Copy link
Member

isuruf commented Dec 5, 2021

comment:40

Can you add primesieve conda package too?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 5, 2021

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

8ef586aprimesieve is on conda, too

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 5, 2021

Changed commit from 5495982 to 8ef586a

@dimpase
Copy link
Member Author

dimpase commented Dec 5, 2021

comment:42

Replying to @isuruf:

Can you add primesieve conda package too?

done.

@vbraun
Copy link
Member

vbraun commented Dec 12, 2021

comment:43

Test fail, e.g.

File "src/sage/plot/plot.py", line 1753, in sage.plot.plot.plot
Failed example:
    plot(lambda x: prime_pi(x), (x, 1, 100), exclude=jumps)
Expected:
    Graphics object consisting of 26 graphics primitives
Got:
    verbose 0 (3839: plot.py, generate_plot_points) WARNING: When plotting, failed to evaluate function at 248 points.
    verbose 0 (3839: plot.py, generate_plot_points) Last error message: 'unable to simplify to float approximation'
    Graphics object consisting of 0 graphics primitives
**********************************************************************
1 item had failures:
   1 of 147 in sage.plot.plot.plot
    [423 tests, 1 failure, 28.78 s]
----------------------------------------------------------------------
sage -t --warn-long 47.7 --random-seed=57011699320160764553253619770787736483 src/sage/plot/plot.py  # 1 doctest failed

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 13, 2021

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

2b61474spkg primecountpy
687687buse primecountpy instead of built-in cython interface
9f6c4a1remove primecount from various lists of optional spkgs
de7de85cython is not in toolchain!
43d481bbump to 0.1.0
f66821bcysignals are needed
ae37b4edeprecate sage.interfaces.primecount, not just remove
d3ae5f4primecount is on conda, too
3a8c7feprimesieve is on conda, too
74b3845allow float inputs for prime_pi

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 13, 2021

Changed commit from 8ef586a to 74b3845

@dimpase
Copy link
Member Author

dimpase commented Dec 13, 2021

comment:45

rebased an added ​74b3845 allow float inputs for prime_pi to deal with the need for prime_pi to accept floats as input.
All the relevant tests pass.

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 13, 2021

comment:46

LGTM.

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 21, 2021

comment:47

Marking this ticket "critical" in light of #33054 because it adds the conda package information

@vbraun
Copy link
Member

vbraun commented Dec 23, 2021

Changed branch from u/dimpase/packages/primecountpy to 74b3845

@vbraun vbraun closed this as completed in 1c8e98f Dec 23, 2021
vbraun pushed a commit that referenced this issue Jun 21, 2023
…ount

    
it has been deprecated for well over a year, time to complete the task
started on #32894.
    
URL: #35754
Reported by: Dima Pasechnik
Reviewer(s): Matthias Köppe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants