Skip to content

Conversation

@DimitriPapadopoulos
Copy link
Contributor

@DimitriPapadopoulos DimitriPapadopoulos commented Oct 27, 2025

In all packages metadata, change licence (deprecated in PEP 639) to licence-files.

@DimitriPapadopoulos DimitriPapadopoulos marked this pull request as draft October 27, 2025 14:04
@DimitriPapadopoulos DimitriPapadopoulos force-pushed the PEP639 branch 2 times, most recently from b7a7aa8 to 7b0108a Compare October 27, 2025 14:13
@thomass-dev thomass-dev self-requested a review October 27, 2025 14:26
@thomass-dev
Copy link
Collaborator

thomass-dev commented Oct 27, 2025

The main reason we did it this way using hatch trigger, was because the build badly integrates the shared files in the build artifacts (in this case, the shared ../LICENSE and the shared ../README.md).

We need to check if your change properly integrates the said files into the build artifacts.

@thomass-dev
Copy link
Collaborator

thomass-dev commented Oct 27, 2025

As far as i can understand, dynamic license (files) and readme are not needed, because the respective files are not generated during the build process.

The properties licence and readme are currently dynamic because they are generated by the hatch hooks (which is the prelude of the build process) by reading the dedicated files from the root of the repository.

@thomass-dev
Copy link
Collaborator

thomass-dev commented Oct 27, 2025

With your PR, hatch==1.15.1 and hatchling==1.27.0, i can confirm my suspicions:

$ python -m build --sdist && tar -tvf dist/skore*.tar.gz | less +G

[...]
-rw-r--r-- 0/0            1069 2020-02-02 01:00 skore-0.0.0+unknown/../LICENSE
-rw-r--r-- 0/0            5746 2020-02-02 01:00 skore-0.0.0+unknown/../README.md
$ python -m build --wheel && unzip -l dist/skore*.whl | less +G

[...]
1069  2020-02-02 00:00   skore-0.0.0+unknown.dist-info/licenses/../LICENSE

which will lead to errors when installing the sdist:

ERROR: Invalid member in the tar file [...]/skore/skore/dist/skore-0.0.0+unknown.tar.gz: '../LICENSE' would be extracted to '/tmp/LICENSE', which is outside the destination

Also for the wheel, i don't think the METADATA are valid either: License-File: ../LICENSE.

@DimitriPapadopoulos
Copy link
Contributor Author

The properties licence and readme are currently dynamic because they are generated by the hatch hooks (which is the prelude of the build process) by reading the dedicated files from the root of the repository.

Why do you need to generate these properties in the the hatch hooks?

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Oct 27, 2025

I guess this could be a bug of the hatch implementation of PEP 639. Whatever the values of readme and license-files, the files should never end up outside of the sdist root directory or outside the wheel licenses directory.

I'll compare with setuptools and flint, and create a hatch issue if they behave differently. I'll check PEP 639 as well, although I doubt it gets into such details.

@DimitriPapadopoulos
Copy link
Contributor Author

From Add license-files key in PEP 639:

  • Parent directory indicators (..) MUST NOT be used.

So I guess there is no way to point to files outside the directory containing pyproject.toml, hence the need for a hatch hook. OK, I'll keep the existing hooks and simply change licenselicense-files. How does this sound?

Update licensing metadata as described in PEP 639.
@DimitriPapadopoulos DimitriPapadopoulos marked this pull request as ready for review October 27, 2025 21:00
Copy link
Collaborator

@thomass-dev thomass-dev left a comment

Choose a reason for hiding this comment

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

Please use cp instead of read/open/write to feed licence-files.

@thomass-dev thomass-dev changed the title chore: PEP 639 compliance chore(all): PEP 639 compliance Oct 28, 2025
@thomass-dev
Copy link
Collaborator

Please use cp instead of read/open/write to feed licence-files.

Please commit this change without force-pushing. This will make the diff easier to review.

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Oct 28, 2025

Finally, should the classifiers from skore/pyproject.toml be copied to other pyproject.toml files?

classifiers = [
"Intended Audience :: Science/Research",
"Intended Audience :: Developers",
"License :: OSI Approved :: MIT License",
"Programming Language :: Python",
"Topic :: Software Development",
"Topic :: Scientific/Engineering",
"Development Status :: 3 - Alpha",
"Operating System :: Microsoft :: Windows",
"Operating System :: POSIX",
"Operating System :: Unix",
"Operating System :: MacOS",
"Programming Language :: Python :: 3",
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: 3.11",
"Programming Language :: Python :: 3.12",
"Programming Language :: Python :: 3.13",
]

More generally, how about striving to make the pyproject.toml files identical, for example tool.ruff.lint.select?

@thomass-dev
Copy link
Collaborator

thomass-dev commented Oct 28, 2025

This is not necessary, because the only visible/searchable package we want is skore/.

@github-actions
Copy link
Contributor

Coverage

Coverage Report for skore/
FileStmtsMissCoverMissing
skore/src/skore
   __init__.py230100% 
   _config.py310100% 
   exceptions.py440%4, 15, 19, 23
skore/src/skore/_sklearn
   __init__.py60100% 
   _base.py1981492%45, 58, 127, 130, 183, 186–187, 189–192, 225, 228–229
   find_ml_task.py610100% 
   types.py27196%28
skore/src/skore/_sklearn/_comparison
   __init__.py70100% 
   feature_importance_accessor.py39294%90, 109
   metrics_accessor.py178398%173, 253, 1215
   report.py1070100% 
   utils.py540100% 
skore/src/skore/_sklearn/_cross_validation
   __init__.py90100% 
   data_accessor.py45393%134, 137, 140
   feature_importance_accessor.py240100% 
   metrics_accessor.py182199%244
   report.py135199%487
skore/src/skore/_sklearn/_estimator
   __init__.py90100% 
   data_accessor.py66198%82
   feature_importance_accessor.py141298%223–224
   metrics_accessor.py356897%200, 202, 209, 300, 369, 373, 388, 423
   report.py167298%448–449
skore/src/skore/_sklearn/_plot
   __init__.py30100% 
   base.py102793%61–62, 200, 224–226, 230
   utils.py770100% 
skore/src/skore/_sklearn/_plot/data
   __init__.py20100% 
   table_report.py185199%706
skore/src/skore/_sklearn/_plot/metrics
   __init__.py60100% 
   confusion_matrix.py70494%92, 100, 122, 230
   feature_importance_display.py672168%88, 121–122, 124, 142–146, 148–155, 158–160, 162
   metrics_summary_display.py80100% 
   precision_recall_curve.py281598%455, 555, 559, 619, 751
   prediction_error.py227597%179, 186, 422, 505, 705
   roc_curve.py294897%387, 510, 515, 616, 621, 625, 694, 834
skore/src/skore/_sklearn/train_test_split
   __init__.py00100% 
   train_test_split.py580100% 
skore/src/skore/_sklearn/train_test_split/warning
   __init__.py80100% 
   high_class_imbalance_too_few_examples_warning.py19194%83
   high_class_imbalance_warning.py200100% 
   random_state_unset_warning.py100100% 
   shuffle_true_warning.py90100% 
   stratify_is_set_warning.py100100% 
   time_based_column_warning.py210100% 
   train_test_split_warning.py30100% 
skore/src/skore/_utils
   __init__.py6266%8, 13
   _accessor.py90396%34, 146, 190
   _environment.py270100% 
   _fixes.py80100% 
   _index.py50100% 
   _logger.py22481%15–17, 19
   _measure_time.py100100% 
   _parallel.py38392%23, 33, 124
   _patch.py13561%21, 23–24, 35, 37
   _progress_bar.py460100% 
   _repr_html.py80100% 
   _show_versions.py380100% 
   _testing.py550100% 
skore/src/skore/project
   __init__.py20100% 
   project.py480100% 
   summary.py75198%120
   widget.py1890100% 
TOTAL402911297% 

Tests Skipped Failures Errors Time
1079 5 💤 0 ❌ 0 🔥 4m 18s ⏱️

@github-actions
Copy link
Contributor

Coverage

Coverage Report for skore-hub-project/
FileStmtsMissCoverMissing
skore-hub-project/src/skore_hub_project
   __init__.py22195%39
   protocol.py310100% 
skore-hub-project/src/skore_hub_project/artifact
   __init__.py00100% 
   artifact.py230100% 
   serializer.py260100% 
   upload.py36488%175, 177–178, 180
skore-hub-project/src/skore_hub_project/artifact/media
   __init__.py50100% 
   data.py220100% 
   feature_importance.py44197%50
   media.py100100% 
   model.py90100% 
   performance.py440100% 
skore-hub-project/src/skore_hub_project/artifact/pickle
   __init__.py20100% 
   pickle.py240100% 
skore-hub-project/src/skore_hub_project/authentication
   __init__.py00100% 
   login.py660100% 
   logout.py40100% 
   token.py240100% 
   uri.py180100% 
skore-hub-project/src/skore_hub_project/client
   __init__.py00100% 
   client.py600100% 
skore-hub-project/src/skore_hub_project/metric
   __init__.py100100% 
   accuracy.py350100% 
   brier_score.py350100% 
   log_loss.py350100% 
   metric.py49197%25
   precision.py610100% 
   r2.py350100% 
   recall.py630100% 
   rmse.py350100% 
   roc_auc.py350100% 
   timing.py90495%52–53, 115–116
skore-hub-project/src/skore_hub_project/project
   __init__.py00100% 
   project.py102397%204, 308, 338
skore-hub-project/src/skore_hub_project/report
   __init__.py30100% 
   cross_validation_report.py64198%204
   estimator_report.py120100% 
   report.py520100% 
TOTAL11861598% 

Tests Skipped Failures Errors Time
139 0 💤 0 ❌ 0 🔥 1m 16s ⏱️

@github-actions
Copy link
Contributor

Coverage

Coverage Report for skore-local-project/
FileStmtsMissCoverMissing
skore-local-project/src/skore_local_project
   __init__.py50100% 
   metadata.py87495%26, 38, 150–151
   project.py102199%244
   storage.py40295%45, 187
TOTAL234797% 

Tests Skipped Failures Errors Time
24 0 💤 0 ❌ 0 🔥 5.747s ⏱️

@github-actions
Copy link
Contributor

Documentation preview @ 5a0b241

@DimitriPapadopoulos
Copy link
Contributor Author

This is not necessary, because the only visible/searchable package we want is skore/.

Makes sense, I would nevertheless suggest:

  • keeping tool.ruff.lint.select identical
  • adding some links such as project.urls.Homepage and project.urls.Repository to all packages

Copy link
Collaborator

@thomass-dev thomass-dev left a comment

Choose a reason for hiding this comment

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

Fine. Thank you for fixing this deprecation.

@thomass-dev thomass-dev merged commit 01dac69 into probabl-ai:main Oct 28, 2025
1 check passed
@DimitriPapadopoulos DimitriPapadopoulos deleted the PEP639 branch October 28, 2025 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants