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

Publish to conda #1786

Merged
merged 72 commits into from
Feb 3, 2022
Merged

Publish to conda #1786

merged 72 commits into from
Feb 3, 2022

Conversation

benoit-cty
Copy link
Contributor

@benoit-cty benoit-cty commented Jan 13, 2022

  • Amélioration technique.
  • Détails :
    • Ajout de la publication du package vers Conda

Ces changements (effacez les lignes ne correspondant pas à votre cas) :

  • Modifient des éléments non fonctionnels de ce dépôt : README et CI.

En détail:

  • Mise à jour du setup.py pour mieux gérer le readme et la license.
  • Mise à jour du README.md pour les explications sur l'utilisation de Conda
  • Ajout du fichier .conda/README.md pour les explications sur la publication Conda
  • Ajout du dossier .conda pour les fichiers spécifique à Conda.
  • Ajout d'une étape de CI pour publier le paquet.
  • J'ai fait plusieurs modifications sur le workflow Github Action :

Quelques conseils à prendre en compte :

Et surtout, n'hésitez pas à demander de l'aide ! :)

@benoit-cty benoit-cty requested a review from sandcha January 13, 2022 15:36
@benoit-cty benoit-cty changed the title Publish to conda WIP: Publish to conda Jan 13, 2022
README.md Outdated Show resolved Hide resolved
@benjello
Copy link
Member

@benoit-cty : me confirmes-tu qu'il n'y a plus de problème de longueur de chemin ?

@benoit-cty
Copy link
Contributor Author

@benoit-cty : me confirmes-tu qu'il n'y a plus de problème de longueur de chemin ?

Oui et non : On ne peut pas intégrer OF-Fr à conda-forge à cause du problème de chemin (voir issue ) mais on peut le pousser vers le channel conda OpenFisca.org en ajoutant un paramètre de build pour builder dans un chemin court au lieu du chemin par défaut.

Et j'ai testé sur mon Windows, l'installation fonctionne. Mais mon user est un nom court : "ben", ça joue surement.

@benoit-cty
Copy link
Contributor Author

Dans setup.py il y a 5 versions de dépendances optionnelles, j'ai réduit cela à 2 pour conda :

  • openfisca-france : uniquement OpenFisca-Core
  • openfisca-france-scipy : OpenFisca-Core + scipy (je suis preneur d'un meilleur nom)
  • openfisca-france-dev : toutes les dépendances
  1. Il y en a une taxipp avec uniquement `pandas mais Pandas n'est pas dans le code OpenFisca donc ça ne me semble pas justifié d'avoir une dépendance vers une librairie qui n'est pas dans le code.
  2. Il y a une section dédié au CASD, comme une des raisons du passage à Conda est de facilité le déploiement sur le CASD ça ne me semble plus nécessaire.

@benoit-cty benoit-cty marked this pull request as ready for review January 17, 2022 15:42
@benoit-cty benoit-cty changed the title WIP: Publish to conda Publish to conda Jan 17, 2022
@benoit-cty
Copy link
Contributor Author

@benjello : Désolé j'ai intégré ta review manuellement sans accepter ton commit, peux-tu revoir ta review ?

Peut-être que je vais finalement réussir à être sur conda-forge également : conda-forge/staged-recipes#17483

Les deux ne sont pas incompatibles : on peut avoir toutes les versions en automatique sur le channel openfisca et les majeure seulement sur conda-forge car c'est une étape manuelles.

.conda/LICENSE.AGPL.txt Outdated Show resolved Hide resolved
.conda/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@sandcha sandcha left a comment

Choose a reason for hiding this comment

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

Merci @benoit-cty 🙌
Voici quelques retours (dont certains déjà transmis à l'oral mais notés ici pour partage du contexte).

.conda/README.md Outdated Show resolved Hide resolved
.conda/meta.yaml Outdated Show resolved Hide resolved
.conda/meta.yaml Show resolved Hide resolved
.conda/meta.yaml Show resolved Hide resolved
.conda/meta.yaml Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@benoit-cty
Copy link
Contributor Author

Prise en compte de la review de @sandcha terminée, j'ai mis à jour la description de la PR pour résumer ce qu'elle contient.

@benoit-cty benoit-cty self-assigned this Jan 20, 2022
@sandcha sandcha requested review from sandcha and benjello January 27, 2022 09:27
Copy link
Member

@benjello benjello left a comment

Choose a reason for hiding this comment

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

Super travail.
La doc est claire et complète.
Je ne suis pas expert sur les autres aspects.
Et il y a une petite correction d'accord.

README.md Outdated Show resolved Hide resolved
.conda/README.md Outdated Show resolved Hide resolved
.conda/README.md Outdated Show resolved Hide resolved
.conda/meta.yaml Show resolved Hide resolved
.github/get_pypi_info.py Show resolved Hide resolved
.github/workflows/workflow.yml Show resolved Hide resolved
.github/workflows/workflow.yml Show resolved Hide resolved
.github/workflows/workflow.yml Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@benoit-cty benoit-cty requested a review from sandcha January 28, 2022 15:00
@benoit-cty benoit-cty linked an issue Jan 28, 2022 that may be closed by this pull request
@benoit-cty
Copy link
Contributor Author

@sandcha J'ai vérifié que le package PyPi se génère toujours bien et que le readme apparaît maintenant bien sur PyPi : https://test.pypi.org/project/OpenFisca-France/104.0.3/
J'ai fait le bump de version. OK pour toi ?

Copy link
Contributor

@sandcha sandcha left a comment

Choose a reason for hiding this comment

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

C'est top ! Merci @benoit-cty 🎉

@sandcha
Copy link
Contributor

sandcha commented Feb 1, 2022

Mini retex sur les opérations conda : en créant un nouvel environnement (ce n'était pas le premier), il a de nouveau fallu exécuter le conda init bash et ouvrir un nouveau terminal pour que l'activate soit possible. 🐼

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
benoit-cty and others added 24 commits February 3, 2022 11:23
Co-authored-by: sandcha <sandcha@users.noreply.github.com>
Co-authored-by: sandcha <sandcha@users.noreply.github.com>
Co-authored-by: Mahdi Ben Jelloul <mahdi.benjelloul@gmail.com>
Co-authored-by: sandcha <sandcha@users.noreply.github.com>
Co-authored-by: sandcha <sandcha@users.noreply.github.com>
Co-authored-by: sandcha <sandcha@users.noreply.github.com>
Co-authored-by: sandcha <sandcha@users.noreply.github.com>
Co-authored-by: sandcha <sandcha@users.noreply.github.com>
Co-authored-by: sandcha <sandcha@users.noreply.github.com>
Co-authored-by: sandcha <sandcha@users.noreply.github.com>
@benoit-cty benoit-cty merged commit 94680fc into master Feb 3, 2022
@benoit-cty benoit-cty deleted the publish-to-conda branch February 3, 2022 10:42
@MattiSG MattiSG mentioned this pull request Sep 10, 2022
5 tasks
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.

Packaging license file
3 participants