Towards PEP621 by IAlibay · Pull Request #3528 · MDAnalysis/mdanalysis

IAlibay · 2022-02-13T02:42:25Z

Fixes #3526

Changes made in this Pull Request:

Adds PEP621 items to pyproject.toml
Updated LICENSE in package to reflect additions to the main repo LICENSE
Removed pypi-description.rst (the 2.0.0 release was fine without it, so I'm assuming whatever made it necessary is no longer needed? - we'll see on release)
Updated setup.py to reflect that we're actually GPL v2+ (using poetry's naming convention here)

TODO:

Add pyproject.toml for testsuite

PR Checklist

Tests?
Docs?
CHANGELOG updated?
Issue raised/referenced?

IAlibay · 2022-02-13T02:45:29Z

package/pyproject.toml

+    {name = 'MDAnalysis Core Developers', email = 'mdanalysis@numfocus.org'}
+]
+requires-python = ">=3.7"
+dependencies = [


I wasn't too sure how to deal with the optional deps here.

Tests -> should I just add MDAnalysisTests as the dep? How do you dynamically set the version number in pyproject.toml?
Extra deps ->

Not sure what the keyword should be here (it's unclear if you can just say "extra").

How do we deal with conda only deps? Does it matter?

conda only deps seems like a bad idea in general, not sure you can do much about them here..

As for dependencies, I think it should just be the mandatory runtime dependencies, and then [project.optional-dependencies] can contain sections for test, doc, and dev tools (I'd suggest just looking at i.e., the SciPy example pyproject.toml and adapting for our purposes, if you aren't already doing that).

Sounds good, for now let's just skip [project.optional-dependencies] since most are conda based, I'll raise an issue on merge of this PR with all the things that can be added at a later date to augment the pyproject.toml.

codecov · 2022-02-13T03:19:15Z

Codecov Report

Base: 94.41% // Head: 94.44% // Increases project coverage by +0.03% 🎉

Coverage data is based on head (aa643aa) compared to base (1f48778).
Patch has no changes to coverable lines.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3528      +/-   ##
===========================================
+ Coverage    94.41%   94.44%   +0.03%     
===========================================
  Files          195      195              
  Lines        25423    25423              
  Branches      3526     3526              
===========================================
+ Hits         24003    24012       +9     
+ Misses        1376     1367       -9     
  Partials        44       44

Impacted Files	Coverage Δ
package/MDAnalysis/due.py	`76.66% <0.00%> (+29.99%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

pep8speaks · 2022-02-13T04:25:28Z

Hello @IAlibay! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

In the file package/setup.py:

Line 573:80: E501 line too long (85 > 79 characters)

In the file testsuite/setup.py:

Line 98:80: E501 line too long (85 > 79 characters)

Comment last updated at 2022-03-27 23:35:25 UTC

package/pyproject.toml

IAlibay · 2022-02-13T04:27:03Z

testsuite/pyproject.toml

+]
+requires-python = ">=3.7"
+dependencies = [
+    'MDAnalysis==2.1.0-dev0',


I'm not super into the idea of having yet-another point where we need to update the version numbers. Is there a way around this?

Hmm, it is indeed pretty ugly from a design standpoint now to have separate main and test packages.

Is there as much of a rush to add a pyproject.toml for the test suite?

It would probably be nice to have the version dynamically set to match the same version of the main package. Not sure how to do that.

A bit out of scope, but what other major Python projects package their test suite entirely separately from the main project? That might be a good reference point for "how should we handle this?"

If we were to start from scratch, I'd almost certainly vote against having separate packages, and instead use one of the popular "lazy loading/lazy download" packages to pull in test assets if there's an internet connection, otherwise skipping and doing a minimal suite only.

For 3.0, getting rid of MDAnalysisTests would be a good aim. One thing to solve would be to keep the correct versions of test files in sync with the tests. Maybe download from the correct tag or commit in the repo? That might get a bit more tricky when you want to add/change test files in a branch... anyway, that would be something to think about and to learn from other packages that presumably already do this.

Which lazy-loading packages do you have in mind?

Do you have an example of a package that pulls in test files when needed?

IAlibay · 2022-02-13T04:28:55Z

@tylerjereddy a lot of this is based on my likely poor understanding of PEP 621 and 631. Any advice / corrections would be greatly appreciated.

tylerjereddy · 2022-02-14T00:41:16Z

package/pyproject.toml

+
+[project]
+name = "MDAnalysis"
+version = "2.1.0-dev0"


Can just do dynamic = ['version'] I think, to avoid having to update this.

tylerjereddy · 2022-02-14T00:45:04Z

package/pyproject.toml

+    {name = 'MDAnalysis Core Developers', email = 'mdanalysis@numfocus.org'}
+]
+requires-python = ">=3.7"
+dependencies = [


conda only deps seems like a bad idea in general, not sure you can do much about them here..

As for dependencies, I think it should just be the mandatory runtime dependencies, and then [project.optional-dependencies] can contain sections for test, doc, and dev tools (I'd suggest just looking at i.e., the SciPy example pyproject.toml and adapting for our purposes, if you aren't already doing that).

package/pyproject.toml

tylerjereddy · 2022-02-14T00:49:18Z

testsuite/pyproject.toml

+
+[project]
+name = "MDAnalysisTests"
+version = "2.1.0-dev0"


I think we can avoid updating this by using dynamic = ['version', 'description'], or similar, based on me cheating and looking at other projects.

tylerjereddy · 2022-02-14T00:54:45Z

testsuite/pyproject.toml

+]
+requires-python = ">=3.7"
+dependencies = [
+    'MDAnalysis==2.1.0-dev0',


Hmm, it is indeed pretty ugly from a design standpoint now to have separate main and test packages.

Is there as much of a rush to add a pyproject.toml for the test suite?

It would probably be nice to have the version dynamically set to match the same version of the main package. Not sure how to do that.

tylerjereddy · 2022-02-14T00:58:07Z

testsuite/pyproject.toml

+]
+requires-python = ">=3.7"
+dependencies = [
+    'MDAnalysis==2.1.0-dev0',


A bit out of scope, but what other major Python projects package their test suite entirely separately from the main project? That might be a good reference point for "how should we handle this?"

If we were to start from scratch, I'd almost certainly vote against having separate packages, and instead use one of the popular "lazy loading/lazy download" packages to pull in test assets if there's an internet connection, otherwise skipping and doing a minimal suite only.

IAlibay · 2022-03-27T22:57:40Z

So long story short, it seems that the latest release of setuptools is essentially enforcing a ton of things related to pyproject.toml which it didn't use to.

To speed things up I'm ditching the addition of a pyproject.toml for testsuite for now. Let's try to get the main package one up to scratch first, then we address testsuite later.

IAlibay · 2022-03-27T23:26:35Z

package/pyproject.toml

+
+[project]
+name = "MDAnalysis"
+dynamic = ['version', 'readme']


@tylerjereddy can I pick your brains on this?

Essentially adding a direct readme entry in the pyproject.toml is complicated because it accesses the README.rst which exists outside of the package directory. So I'm thinking we can possibly just leave it as dynamic since setup.py has set logic for gathering long-description, does that make sense to do?

edit: adding to dynamic still caused issues... not sure what the best solution here is at this point.

package/pyproject.toml

orbeckst

The license update to "GPL v2 or later" is perfect. ✅

IAlibay · 2022-11-04T22:44:55Z

I'm adding this to the 2.4.0 milestone, with the way that things are going we should attempt to be fully pep621 compliant by end of year.

TODOs here are mostly updating things.

IAlibay · 2022-12-07T11:24:58Z

package/pyproject.toml

+    "scikit-learn",
+    "tidynamics>=1.0.0",
+]
+doc = [


missing test entry because passing in a dynamic version for MDAnalysisTests doesn't really work...

IAlibay · 2022-12-07T12:36:39Z

@MDAnalysis/coredevs this is good to go as needed for 2.4.0, I recommend using scipy's pyproject.toml as a template to check against during review: https://github.com/scipy/scipy/blob/main/pyproject.toml

IAlibay · 2022-12-15T17:40:41Z

@MDAnalysis/coredevs can I get a review here please? This is stalling the 2.4.0 release

IAlibay · 2022-12-15T18:24:09Z

Ta, two PRs to go.

Initial attempt at PEP 621 for main MDA package

82e0328

IAlibay commented Feb 13, 2022

View reviewed changes

IAlibay added 4 commits February 13, 2022 02:47

version should be string

6e4c9b0

toml tables not python

40be8aa

switch to project.urls, hope the whitespace works

90f5616

quoted keys _should_ be fine?

5b44eb9

pyproject.toml for testsuite

3f5b303

IAlibay commented Feb 13, 2022

View reviewed changes

package/pyproject.toml Show resolved Hide resolved

IAlibay commented Feb 13, 2022

View reviewed changes

uncomment test

fe05f9f

IAlibay requested a review from tylerjereddy February 13, 2022 04:27

tylerjereddy added the maintainability label Feb 14, 2022

tylerjereddy reviewed Feb 14, 2022

View reviewed changes

IAlibay mentioned this pull request Feb 22, 2022

Release 2.1.0 #3446

Closed

5 tasks

IAlibay added 3 commits March 27, 2022 23:40

Merge branch 'develop' into issue-3526

ad006fb

Remove testsuite pyproject.toml for now, dynamic version

e118a12

try to circumvent disutil issues with manifest

fda7634

IAlibay added 5 commits March 28, 2022 00:00

Add trove-classifiers for better testing of pyproject.toml classifiers

b00d4c5

temp disable readme from pyproject.toml

3153109

don't forget packaging

755a65e

move readme to dynamic section

fa6063a

revert manifest

a7ed38b

IAlibay commented Mar 27, 2022

View reviewed changes

remove readme completely for now

fd7ccda

RMeli reviewed Mar 30, 2022

View reviewed changes

package/pyproject.toml Outdated Show resolved Hide resolved

orbeckst reviewed Apr 4, 2022

View reviewed changes

IAlibay mentioned this pull request Aug 29, 2022

Add upper pins for some core dependencies on deployment #3796

Closed

IAlibay self-assigned this Nov 4, 2022

IAlibay added this to the 2.4.0 milestone Nov 4, 2022

IAlibay added 3 commits December 7, 2022 09:43

Merge branch 'develop' into issue-3526

81926ce

various pyproject.toml fixups

2977ddb

Add changelog entry

6c1b3ab

IAlibay commented Dec 7, 2022

View reviewed changes

IAlibay and others added 2 commits December 7, 2022 11:47

fix typo

f638e1b

Merge branch 'develop' into issue-3526

68d4735

Merge branch 'develop' into issue-3526

aa643aa

richardjgowers approved these changes Dec 15, 2022

View reviewed changes

IAlibay merged commit b3b907b into develop Dec 15, 2022

IAlibay deleted the issue-3526 branch December 15, 2022 18:23

Conversation

IAlibay commented Feb 13, 2022

PR Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Feb 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pep8speaks commented Feb 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated at 2022-03-27 23:35:25 UTC

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IAlibay commented Feb 13, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IAlibay commented Mar 27, 2022

Uh oh!

IAlibay Mar 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

orbeckst left a comment

Choose a reason for hiding this comment

Uh oh!

IAlibay commented Nov 4, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IAlibay commented Dec 7, 2022

Uh oh!

IAlibay commented Dec 15, 2022

Uh oh!

IAlibay commented Dec 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov bot commented Feb 13, 2022 •

edited

Loading

pep8speaks commented Feb 13, 2022 •

edited

Loading

IAlibay Mar 27, 2022 •

edited

Loading