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

Remplace setup.py par pyproject.toml #2298

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft

Remplace setup.py par pyproject.toml #2298

wants to merge 13 commits into from

Conversation

MattiSG
Copy link
Member

@MattiSG MattiSG commented May 5, 2024

  • Amélioration technique.
  • Zones impactées : setup.py.
  • Détails :
    • Remplace les métadonnées du paquet dans setup.py par le format de fichier plus moderne pyproject.toml

Ces changements :

  • Mettent à jour un élément de métadonnée pour aligner avec les bonnes pratiques courantes de l'écosystème Python.

Besoin d'aide

  • Relecture et validation de l'impact sur Conda — j'ai fait une modification théorique mais ne suis pas sûr qu'elle soit suffisante et je ne sais pas tester le packaging Conda. J'espère que @benoit-cty pourra contribuer.
  • Estimation de l'impact de passer la version minimale de Core à la v41. J'espère que @benjello ou @eraviart pourront contribuer.

Nous pourrons déterminer le type de changement (mineur ou majeur) en fonction de l'évaluation des points ci-dessus.

@MattiSG MattiSG requested a review from benoit-cty May 5, 2024 20:20
Copy link
Contributor

@benoit-cty benoit-cty left a comment

Choose a reason for hiding this comment

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

Conda update seems to work, but I will confirm later as we have an internet outage here...

.github/get_minimal_version.py Outdated Show resolved Hide resolved
.github/is-version-number-acceptable.sh Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
.conda/meta.yaml Outdated Show resolved Hide resolved
.conda/meta.yaml Outdated Show resolved Hide resolved
{% for req in data.get('dev_requirements', []) %}
- {{ req }}
{% endfor %}
# {% for req in pyproject.get('project').get('optional-dependencies').get('dev') %}
Copy link
Contributor

Choose a reason for hiding this comment

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

J'ai passé du temps à essayer de faire fonctionner le template Jinja, sans succès. Donc j'ai mis en dur les dépendances de dev.

Copy link
Member Author

Choose a reason for hiding this comment

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

Faut-il vraiment inclure les dev requirements dans la version packagée Conda ?! J'avais cru comprendre que le paquet Conda est là pour être exécutée et non pas être éditable. Si c'est bien le cas, pourquoi aurait-elle besoin des linters, par exemple ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Je pensais qu'il était fait pour les contributeurs Windows comme l'IPP, mais je crois savoir qu'ils utilisent pip dans Conda. Je l'avais fait pour le CASD mais ils sont passés à un proxy pip qui est plus pratique à utiliser.
Donc tu peux retirer cette partie dev je pense oui.

benoit-cty
benoit-cty previously approved these changes May 6, 2024
@MattiSG
Copy link
Member Author

MattiSG commented May 7, 2024

@benoit-cty merci de ne pas utiliser cette branche qui a une pull request ouverte pour faire des tests sur la CI mais d'utiliser une branche temporaire et d'intégrer seulement la version finale, ce afin d'éviter de notifier à chaque commit et de forcer les autres personnes qui travaillent sur la branche à mettre à jour leur propre code fréquemment 🙂

@MattiSG
Copy link
Member Author

MattiSG commented May 7, 2024

Suite aux derniers changements effectués, une nouvelle erreur apparaît :

ImportError: No country package has been detected on your environment. If your country package is installed but not detected, please use the --country-package option.

Je n'aurai pas le temps de prioriser ce remplacement de fichier de métadonnées avant un moment vu la hausse progressive de complexité sur la proposition initiale.

@MattiSG MattiSG dismissed benoit-cty’s stale review May 7, 2024 12:39

De nombreux changements ont été effectués depuis la revue positive, une nouvelle revue sera nécessaire

@MattiSG MattiSG marked this pull request as draft May 7, 2024 12:39
@benoit-cty
Copy link
Contributor

Repris dans #2369 qui a été mergée.
Peut-on fermer cette PR ?

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