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.

Sign up for GitHub

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

Jump to bottom

timeseries fonctionalité essentielle #12

Merged
merged 27 commits into from
Feb 20, 2023
Merged

timeseries fonctionalité essentielle #12

merged 27 commits into from
Feb 20, 2023

Conversation

Beauprel
Copy link
Contributor

@Beauprel Beauprel commented Feb 6, 2023 •
edited
Loading

  • fichier timeseries. py : fonction timeseries
  • fichier util_fcts.py : fonctions utilitaires

Fonctionnalité

  • détecte les ensembles Xclim percentiles et statistiques
  • assigne les éléments textuels des graphiques à des attributs des objets Xarray (avec valeurs par défaut)
  • convertis calendriers CFtime à calendriers standard
  • plusieurs options de légende

Notes

  • pas encore de travail effectué sur les couleurs, style, traduction, logo Ouranos
  • voir Issues pour liste de problèmes potentiels et limitations

Beauprel and others added 19 commits January 12, 2023 16:55
@Beauprel
creation fichiers
2e46734
@Beauprel
w.i.p.
6c31391
@Beauprel
fonctions utilitaires de base
5f75c60
@Beauprel
ajout fonctionnalite basic Dataset
b5af3d0
@Beauprel
started ensemble functionality ad created sort_lines fct
48f2636
@Beauprel
ensemble function linestyle improvements
b07a7a3
@Beauprel
implemented attributes pull from Xarray objects
53591ac
@Beauprel
timeseries todo list updated
7b332e4
@Beauprel
restructuration de la fonction pour accepter un dictionnaire de Datasets
af16da4
@Beauprel
functionality for var, dim ensembles, regular DS and regular DA
d157f82
@Beauprel
minor improvements, scripts to get data
e219842
@Beauprel
first functionality tests
d00c469
@Beauprel
legend inclues patches, many other upgrades
7b7eeff
@Beauprel
lat,on out of title, label bug fixed, in-plot legend first (buggy) ve…
4eb742f 
…rsion
@Beauprel
split_legend func, docstrings updated, non-ensemble DS legend fixed
0d556ed
@Beauprel
Delete get_example_data.py
d2688b4
@Beauprel
Delete timeseries_test.py
18328e9
@Beauprel
split_legend func, docstrings updated, non-ensemble DS legend fixed
f06fd81
@Beauprel
Merge remote-tracking branch 'origin/alexis_mpl_1' into alexis_mpl_1
f5047bc
@Beauprel Beauprel self-assigned this Feb 6, 2023
@sarahclaude sarahclaude self-requested a review February 6, 2023 19:10
juliettelavoie
juliettelavoie reviewed Feb 7, 2023
View reviewed changes
Copy link
Contributor

@juliettelavoie juliettelavoie left a comment

Choose a reason for hiding this comment

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

Première ronde de commentaires:
En général, c'est très bien ! La plupart de mes commentaires pour l'instant sont sur le style. Je n'ai pas eu le temps de tester encore.

spirograph/matplotlib/timeseries.py Outdated Show resolved Hide resolved
spirograph/matplotlib/timeseries.py Outdated Show resolved Hide resolved
spirograph/matplotlib/timeseries.py Outdated Show resolved Hide resolved
spirograph/matplotlib/timeseries.py Outdated Show resolved Hide resolved
spirograph/matplotlib/timeseries.py Outdated Show resolved Hide resolved
spirograph/matplotlib/timeseries.py Outdated Show resolved Hide resolved
spirograph/matplotlib/timeseries.py Outdated Show resolved Hide resolved
spirograph/matplotlib/timeseries.py Outdated Show resolved Hide resolved
spirograph/matplotlib/timeseries.py Outdated Show resolved Hide resolved
spirograph/matplotlib/util_fcts.py Outdated Show resolved Hide resolved
sarahclaude
sarahclaude reviewed Feb 8, 2023
View reviewed changes
Copy link
Collaborator

@sarahclaude sarahclaude left a comment

Choose a reason for hiding this comment

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

Je n'ai pas encore testé la fonction.
Donc, tout ce qui serait à ajouter à la fonction time_series serait une option de logo, la traduction et les couleurs RCP / SSP?

spirograph/matplotlib/timeseries.py Outdated Show resolved Hide resolved
spirograph/matplotlib/util_fcts.py Outdated Show resolved Hide resolved
spirograph/matplotlib/util_fcts.py Outdated Show resolved Hide resolved
spirograph/matplotlib/util_fcts.py Outdated Show resolved Hide resolved
spirograph/matplotlib/util_fcts.py Outdated Show resolved Hide resolved
spirograph/matplotlib/util_fcts.py Outdated Show resolved Hide resolved
spirograph/matplotlib/util_fcts.py Outdated Show resolved Hide resolved
spirograph/matplotlib/util_fcts.py Outdated Show resolved Hide resolved
spirograph/matplotlib/util_fcts.py Outdated Show resolved Hide resolved
spirograph/matplotlib/timeseries.py Outdated Show resolved Hide resolved
@Beauprel
Copy link
Contributor Author

Beauprel commented Feb 9, 2023

Je n'ai pas encore testé la fonction. Donc, tout ce qui serait à ajouter à la fonction time_series serait une option de logo, la traduction et les couleurs RCP / SSP?

Reste aussi à ajouter la fonctionnalité pour les Datasets avec centiles

@Beauprel
- adressed comments from PR-12
7aa5916 
- Simplified how xlabel 'time' is assigned
- Changed default-setting method for use_attrs
- Added support for Dataset with multiple percentile ensembles
@Beauprel
- fixed label bugs
abb2f2c 
- minor name updates
juliettelavoie
juliettelavoie reviewed Feb 13, 2023
View reviewed changes
spirograph/matplotlib/timeseries.py Outdated Show resolved Hide resolved
spirograph/matplotlib/timeseries.py Outdated Show resolved Hide resolved
spirograph/matplotlib/timeseries.py Outdated Show resolved Hide resolved
spirograph/matplotlib/timeseries.py Outdated Show resolved Hide resolved
spirograph/matplotlib/timeseries.py Outdated Show resolved Hide resolved
spirograph/matplotlib/timeseries.py Outdated Show resolved Hide resolved
spirograph/matplotlib/util_fcts.py Outdated Show resolved Hide resolved
spirograph/matplotlib/timeseries.py Outdated Show resolved Hide resolved
spirograph/matplotlib/timeseries.py Outdated Show resolved Hide resolved
spirograph/matplotlib/timeseries.py Outdated Show resolved Hide resolved
@Beauprel
- labelling system for non-ensemble Datasets
dae5098 
- minor bugs fixed
@Beauprel
- renamed files
8adde57 
- added support for climate ensembles
- added utils plot_realizations() and fill_between_label()
- etc.
@Beauprel
- changed import method
7a0f102 
- moved function from utils to plot
juliettelavoie
juliettelavoie approved these changes Feb 20, 2023
View reviewed changes
Copy link
Contributor

@juliettelavoie juliettelavoie left a comment

Choose a reason for hiding this comment

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

super! Je pense qu'on a une bonne base!

spirograph/matplotlib/plot.py Outdated Show resolved Hide resolved
spirograph/matplotlib/plot.py Outdated Show resolved Hide resolved
spirograph/matplotlib/plot.py Outdated Show resolved Hide resolved
spirograph/matplotlib/plot.py Outdated Show resolved Hide resolved
spirograph/matplotlib/plot.py Outdated Show resolved Hide resolved
spirograph/matplotlib/plot.py Outdated Show resolved Hide resolved
@Beauprel @juliettelavoie
Apply suggestions from code review
d646e20 
Co-authored-by: juliettelavoie <juliette.lavoie@hotmail.ca>
@Beauprel Beauprel merged commit a061187 into main Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sarahclaude sarahclaude sarahclaude left review comments

@juliettelavoie juliettelavoie juliettelavoie approved these changes

Assignees

@Beauprel Beauprel

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Beauprel @juliettelavoie @sarahclaude