Skip to content

Add pyproject.toml, update installation instructions for modern Python packaging, fix docbuild, update CI scripts, add Cython 3 support#139

Merged
dimpase merged 23 commits intosagemath:masterfrom
mkoeppe:pyproject_toml
Oct 6, 2023
Merged

Conversation

@mkoeppe
Copy link
Copy Markdown
Contributor

@mkoeppe mkoeppe commented Oct 4, 2023 •
edited
Loading

This PR started out with minimal changes, replacing the outdated use of setup_requires.

Fixes #93
(with a cleaner solution for augmenting the build-time path than the stalled PR #104).

videlec
videlec reviewed Oct 5, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@videlec videlec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

Comment thread _custom_build_meta.py
Copy link
Copy Markdown
Collaborator

@videlec videlec Oct 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you make the file name starts with an underscore here?

Copy link
Copy Markdown
Contributor Author

@mkoeppe mkoeppe Oct 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reason, feel free to rename

Copy link
Copy Markdown
Contributor Author

@mkoeppe mkoeppe Oct 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want me to just remove the underscore, or do you have other naming preferences?

Copy link
Copy Markdown
Collaborator

@videlec videlec Oct 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing beyond following standard practice.

Copy link
Copy Markdown
Contributor Author

@mkoeppe mkoeppe Oct 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're making it sound as if you think my PR was not following standard practice.

@dimpase
Copy link
Copy Markdown
Member

dimpase commented Oct 5, 2023

@videlec - what is the status of cypari2 vs Cython 3? You probably noticed that Sage has moved to Cython 3 (and it works)

dimpase
dimpase reviewed Oct 5, 2023
View reviewed changes
Comment thread pyproject.toml Outdated
@dimpase
Copy link
Copy Markdown
Member

dimpase commented Oct 5, 2023 •
edited
Loading

@mkoeppe - can you add https://github.com/sagemath/sage/blob/develop/build/pkgs/cypari/patches/cython3-legacy.patch and drop the <3 constraint on Cython version added in c473052 ?

PS. OK, I've done it myself

@videlec
Copy link
Copy Markdown
Collaborator

videlec commented Oct 6, 2023

I think the workflow should include both Cython>=3 and Cython<3.

@dimpase
Copy link
Copy Markdown
Member

dimpase commented Oct 6, 2023

I think the workflow should include both Cython>=3 and Cython<3.

This would be a bigger change than this PR. A new issue?

@dimpase
Copy link
Copy Markdown
Member

dimpase commented Oct 6, 2023

@mkoeppe - what this ERROR: Could not find a version that satisfies the requirement setuptools (from versions: none)
stuff mean? I have no clue.

@dimpase
Copy link
Copy Markdown
Member

dimpase commented Oct 6, 2023

Does make build run an isolated Python environment, so that the previously run pip install has no effect?
I don't understand.

@dimpase
Copy link
Copy Markdown
Member

dimpase commented Oct 6, 2023

@mkoeppe - it doesn't fly. make build does not want to accept system-wide installed setuptools. full stop.

@dimpase
Copy link
Copy Markdown
Member

dimpase commented Oct 6, 2023

OK, we finally get a working build and testing, doc building is broken:

Running Sphinx v7.2.6
WARNING: Invalid configuration value found: 'language = None'. Update your configuration to a valid language code. Falling back to 'en' (English).
making output directory... done
WARNING: html_static_path entry '_static' does not exist
building [mo]: targets for 0 po files that are out of date
writing output... 
building [html]: targets for 7 source files that are out of date
updating environment: [new config] 7 added, 0 changed, 0 removed

Exception occurred:
  File "/home/runner/work/cypari2/cypari2/tvenv/lib/python3.11/site-packages/sphinx/ext/extlinks.py", line 108, in role
    title = caption % part
            ~~~~~~~~^~~~~~
TypeError: not all arguments converted during string formatting
The full traceback has been saved in /tmp/sphinx-err-dpid6l08.log, if you want to report the issue to the developers.

@dimpase
Copy link
Copy Markdown
Member

dimpase commented Oct 6, 2023

this is due to broken format strings in extlinks in docs/source/conf.py. fixing...

@dimpase
Copy link
Copy Markdown
Member

dimpase commented Oct 6, 2023

OK, builds/ytests/docs with Pari 2.15.4 pass, ones with Pari 2.11 and 2.13 take much more time, no idea why.

@mkoeppe
Copy link
Copy Markdown
Contributor Author

mkoeppe commented Oct 6, 2023

Let's get rid of make build. This makes no sense any more.

@mkoeppe
Copy link
Copy Markdown
Contributor Author

mkoeppe commented Oct 6, 2023

All done, please merge

@dimpase dimpase merged commit 7386a9f into sagemath:master Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dimpase dimpase dimpase left review comments

@videlec videlec videlec left review comments

Assignees

@dimpase dimpase

@videlec videlec

@mkoeppe mkoeppe

Labels

None yet

Projects

None yet

Milestone

2.1.4

Development

Successfully merging this pull request may close these issues.

Needs pyproject.toml to declare build-time dependency on cysignals

3 participants

@mkoeppe @dimpase @videlec