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

Add current xclim testdata #1

Merged
merged 1 commit into from
Oct 5, 2020
Merged

Add current xclim testdata #1

merged 1 commit into from
Oct 5, 2020

Conversation

Zeitsperre
Copy link
Contributor

Mimicked the existing structure and scripts present in https://github.com/pydata/xarray-data, except the file-structure is kept intact. Calculated the md5 checksums as xarray-data does as well. We should perform the same check for json files as well.

@Zeitsperre Zeitsperre added the enhancement New feature or request label Oct 5, 2020
@Zeitsperre Zeitsperre requested a review from huard October 5, 2020 20:45
@Zeitsperre Zeitsperre self-assigned this Oct 5, 2020
@Zeitsperre Zeitsperre merged commit 6de3623 into main Oct 5, 2020
@Zeitsperre Zeitsperre deleted the add_data branch October 5, 2020 21:03
@huard
Copy link
Contributor

huard commented Oct 5, 2020

@tlvu Est-ce que tu voyais ça comme ça aussi pour raven ?

@tlvu
Copy link

tlvu commented Oct 6, 2020

@huard J'ai réfléchi à la question, je suis 50-50 pour sortir les testdata dans un autre repo.

Le plus gros avantage de garder les testdata à part c'est que ça prévient le main repo d'être gros à cause des testdata.

En retour, le plus gros désavantage que les testdata sont à part c'est la synchro du code et les testdata (faut merger les testdata avant de merger le code et durant le dev cycle, il faut référencer la branch devel des testdata) et le support de revenir en arrière avec le code (on ne peut plus effacer ou changer les testdata existant, sinon ca brise les anciens version du code).

Pour le partage des testdata, il y a aucune différence, si un autre repo veut les avoir, pour lui les testdata proviennent toujours d'un repo externe, qu'ils soient séparé du code ou non.

Ceci dit, avoir les testdata dans un repo externe n'est pas une invitation à y mettre n'importe quoi non plus. Il faut les simpliier (pour que ce soit facile à comprendre ces testdata) et il faut les avoir le plus petit possible (pour ne pas exploser la grosseur du repo).

Donc penses y et laisse moi savoir si tu veux la même chose pour Raven. Selon moi, à moins qu'on anticipe une augmentation significative de la taille des testdata, laisser les tel quel dans le main repo simplifie les choses. De toute façons enlever les testdata maintenant ne va pas rapetisser le repo vu qu'ils sont déjà dans l'historique de git.

Pour mon sync des testdata sur Thredds, ça n'a aucun impact. Le script voit qu'il doit sortir un dossier d'un repo git pour les mettre sur Thredds, peut importe quel repo git il s'agit.

@huard
Copy link
Contributor

huard commented Oct 6, 2020

Ok. Gardons le statut-quo pour l'instant dans Raven et ajoutons simplement une fonction pour faciliter l'accès.

@huard
Copy link
Contributor

huard commented Nov 24, 2020

On sépare le wrapper python pour Raven du serveur WPS. On va donc avoir besoin d'un accès aux mêmes données de tests pour deux repos différents.

@tlvu
Copy link

tlvu commented Nov 24, 2020

On sépare le wrapper python pour Raven du serveur WPS. On va donc avoir besoin d'un accès aux mêmes données de tests pour deux repos différents.

Ok, mais pour le nouveau wrapper python, les données de test seront pour lui toujours d'un repo externe, que ce soit le Raven WPS repo ou un autre raven-testdata repo, c'est la même chose.

Comme écrit ici #1 (comment) les 2 solutions ont tous les pour et contre donc pas clair le net avantage de le sortir.

Mais je m'y tiens pas trop. Il y a peut-être d'autres raisons auquel j'y pense pas. Tu es mieux placé que moi pour le savoir.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants