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

default method dans extract_dataset #57

Closed
juliettelavoie opened this issue Aug 30, 2022 · 9 comments · Fixed by #58
Closed

default method dans extract_dataset #57

juliettelavoie opened this issue Aug 30, 2022 · 9 comments · Fixed by #58

Comments

@juliettelavoie
Copy link
Contributor

Generic Issue

For tasmax, the default method in resample is max. This makes sense for hourly to daily resampling, but isn't what the user expects for daily to monthly. In that case, the user would have to explicitely call method='mean'.

When resample is called by extract_dataset, there is no way to set the method.

Either resample needs to do a better job at setting the default or method_per_variable need to be an argument of 'extract_dataset`.

@aulemahal
Copy link
Collaborator

aulemahal commented Aug 30, 2022

En fait, me semble que max ce n'est jamais la manière de resampler tasmax ? Il me semble que de passer de tas@1h à tasmax@1d c'est encore impossible?

@juliettelavoie
Copy link
Contributor Author

mais, on ne parle pas de tas. Pour tasmax hourly à tasmax daily, max est la bonne manière non ? Le maximum de la journée, c'est le maximum des maximums hourly (pas la moyenne des max hourly).

@RondeauG
Copy link
Contributor

RondeauG commented Aug 30, 2022

Les méthodes qui sont là sont pensées pour le subdaily, où en effet ça serait max (dans la situation hypothétique où on aurait un tasmax sous-journalier, ce qui n'est à peu près jamais le cas).

@RondeauG
Copy link
Contributor

Cela dit plutôt qu'ajouter un argument, j'irais plutôt en faveur de trouver un moyen relativement simple pour un User de supplanter resampling_methods.json.

@juliettelavoie
Copy link
Contributor Author

juliettelavoie commented Aug 30, 2022

dans resampling_method.json, on pourrait avoir

{'any':
  {
	"sfcWindfromdir": "wind_direction",
	"sfcWind": "wind_direction",
	"uas": "wind_direction",
	"vas": "wind_direction"
  },
'D':
  {
	"tasmin": "min",
	"tasmax": "max"
  }
}

@aulemahal
Copy link
Collaborator

Et dans extract_dataset on peut facilement avoir un argument resample qui serait un override de resampling_methods.

@RondeauG
Copy link
Contributor

Je vois en fait qu'on avait déjà un argument method à resample, il n'est simplement pas utilisé

@aulemahal
Copy link
Collaborator

Je note que xs.utils.CV.resampling_methods expose ses données avec .dictet donc, une fois xscen importé, on peut faire les modifications qu'on veut.

@juliettelavoie
Copy link
Contributor Author

ok je peux ouvrir une PR pour faire les modifs.

@juliettelavoie juliettelavoie mentioned this issue Aug 30, 2022
6 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 a pull request may close this issue.

3 participants