entrouvert
/
chrono
14
1
Fork 0
Code Issues Pull Requests 6 Releases Wiki Activity

découpage de get_all_slots() (#76335) #67

Merged
bdauvergne merged 5 commits from wip/76335-Decoupage-de-get-all-slots-en-de into main 2023-05-30 22:24:58 +02:00
Conversation 6 Commits 5 Files Changed 7 +1545 -328
bdauvergne commented 2023-04-06 12:54:23 +02:00
Owner
Copy Link

1ère étape bouger get_all_slots() et ses amis dans la classe Agenda.

1ère étape bouger get_all_slots() et ses amis dans la classe Agenda.
bdauvergne force-pushed wip/76335-Decoupage-de-get-all-slots-en-de from 208ee1a111 to 8c164a6020 2023-04-06 12:56:44 +02:00 Compare
bdauvergne force-pushed wip/76335-Decoupage-de-get-all-slots-en-de from 8c164a6020 to 1133be3e3b 2023-04-06 12:58:29 +02:00 Compare
bdauvergne force-pushed wip/76335-Decoupage-de-get-all-slots-en-de from 0d40104eb2 to 55499a2a28 2023-04-06 20:53:56 +02:00 Compare
bdauvergne force-pushed wip/76335-Decoupage-de-get-all-slots-en-de from 55499a2a28 to f97da02549 2023-04-07 10:23:23 +02:00 Compare
bdauvergne force-pushed wip/76335-Decoupage-de-get-all-slots-en-de from f97da02549 to ab495ea81e 2023-04-07 14:36:18 +02:00 Compare
bdauvergne force-pushed wip/76335-Decoupage-de-get-all-slots-en-de from ab495ea81e to 03798fe212 2023-04-07 14:55:01 +02:00 Compare
bdauvergne force-pushed wip/76335-Decoupage-de-get-all-slots-en-de from 03798fe212 to 5ab306735a 2023-04-07 15:39:30 +02:00 Compare
bdauvergne force-pushed wip/76335-Decoupage-de-get-all-slots-en-de from 5ab306735a to 1aaef3ba4c 2023-04-07 15:41:30 +02:00 Compare
bdauvergne changed title from WIP: découpage de get_all_slots() (#76335) to découpage de get_all_slots() (#76335) 2023-04-07 15:42:38 +02:00
bdauvergne commented 2023-04-07 16:03:56 +02:00
Author
Owner
Copy Link

Pas de découpage finalement mais un peu de rangement avec get_min_datetime()/get_max_datetime() et get_all_slots() qui atterrissent dans les méthodes Agenda où il aurait du être.

Au final ce n'est pas évident de trouver les choses communes, mais en dupliquant une fois puis en revenant dessus on devrait y arriver, plus tard; get_free_time() ne servira de toute façon qu'au code spécifique ANTS.

SharedTimePeriod.get_time_slots() pourrait par contre être reconstruit facilement au dessus de SharedTimePerdio.get_intervals() en plus clair, on verra plus tard.

J'ai importé les outils de test de #17685 et complété l'implémentation de IntervalSet pour les besoins de get_free_time().

Pas de découpage finalement mais un peu de rangement avec get_min_datetime()/get_max_datetime() et get_all_slots() qui atterrissent dans les méthodes Agenda où il aurait du être. Au final ce n'est pas évident de trouver les choses communes, mais en dupliquant une fois puis en revenant dessus on devrait y arriver, plus tard; get_free_time() ne servira de toute façon qu'au code spécifique ANTS. SharedTimePeriod.get_time_slots() pourrait par contre être reconstruit facilement au dessus de SharedTimePerdio.get_intervals() en plus clair, on verra plus tard. J'ai importé les outils de test de #17685 et complété l'implémentation de IntervalSet pour les besoins de get_free_time().
ecazenave commented 2023-04-18 16:53:22 +02:00
Owner
Copy Link

(relecture non exhaustive)

"8c390b9e315f dd helper method to build meeting agenda" : quelque remarques sur les add_*, mais globalement ok parce que c'est compréhensible lorsqu'on lit un test qui utilise une de ces méthode.

Par contre pas fan du tout des build_*, dont on a en gros aucune idée de ce que ça fait excatement, ce qui obligera le relecteur d'un test à interrompre sa lecture pour aller comprendre ce qui se passe dans la méthode build_machin.

1aaef3ba4cf815ec74 : l'unique test utilise un agenda virtuel (plutôt qu'un agenda simple), vraiment pas idéal pour comprendre, possible d'ajouter des petits tests sur des cas bateaux ?

Global : les typing.Whatever, j'aimerais qu'on ait une discussion globale à ce sujet qu'on s'accorde ou pas pour introduire ce genre de choses dans Publik, plutôt que voir ça introduit ici ou là par ceux à qui ça plaît.

(relecture non exhaustive) "8c390b9e315f dd helper method to build meeting agenda" : quelque remarques sur les add_*, mais globalement ok parce que c'est compréhensible lorsqu'on lit un test qui utilise une de ces méthode. Par contre pas fan du tout des build_*, dont on a en gros aucune idée de ce que ça fait excatement, ce qui obligera le relecteur d'un test à interrompre sa lecture pour aller comprendre ce qui se passe dans la méthode build_machin. 1aaef3ba4cf815ec74 : l'unique test utilise un agenda virtuel (plutôt qu'un agenda simple), vraiment pas idéal pour comprendre, possible d'ajouter des petits tests sur des cas bateaux ? Global : les typing.Whatever, j'aimerais qu'on ait une discussion globale à ce sujet qu'on s'accorde ou pas pour introduire ce genre de choses dans Publik, plutôt que voir ça introduit ici ou là par ceux à qui ça plaît.
bdauvergne commented 2023-04-18 18:45:40 +02:00
Author
Owner
Copy Link

Par contre pas fan du tout des build_*, dont on a en gros aucune idée de ce que ça fait excatement, ce qui obligera le relecteur d'un test à interrompre sa lecture pour aller comprendre ce qui se passe dans la méthode build_machin.

Clairement c'est totalement l'effet contraire de ce que je souhaite qui est qu'on doit comprendre intuitivement immédiatement le contexte dans lequel on se trouve concernant les données; donc bien content que tu le dises.

Si j'enlève toute magie, c'est à dire si la description complète de l'agenda, rendu succincte, est présente dans les appels ce serait plus acceptable ? Genre:

agenda = build_virtual_agenda('''
Agenda 1
 Bureau
  monday-wednesday;08:00-12:00,14:00-17:00
  thurday-friday:10:00-14:00
Agenda 2
 Bureau 
  monday-friday;08:00-12:00,14:00-17:00
''')
# agenda._agenda_1.bureau est défini

1aaef3ba4cf815ec74 : l'unique test utilise un agenda virtuel (plutôt qu'un agenda simple), vraiment pas idéal pour comprendre, possible d'ajouter des petits tests sur des cas bateaux ?

Ok.

Global : les typing.Whatever, j'aimerais qu'on ait une discussion globale à ce sujet qu'on s'accorde ou pas pour introduire ce genre de choses dans Publik, plutôt que voir ça introduit ici ou là par ceux à qui ça plaît.

À part typing.NamedTuple qui fait quelque chose mais est strictement équivalent à collections.namedtuple() en étant plus lisible, les autres c'est purement de la doc, particulièrement pour les unions je trouve ça pratique de pouvoir décrire qu'un truc peut-être ceci ou cela dans des fonctions qui sont faites pour faciliter l'écriture des tests et leur lecture. Ensuite c'est dans les tests, je pense qu'on peut être plus souple là.

> Par contre pas fan du tout des build_*, dont on a en gros aucune idée de ce que ça fait excatement, ce qui obligera le relecteur d'un test à interrompre sa lecture pour aller comprendre ce qui se passe dans la méthode build_machin. Clairement c'est totalement l'effet contraire de ce que je souhaite qui est qu'on doit comprendre intuitivement immédiatement le contexte dans lequel on se trouve concernant les données; donc bien content que tu le dises. Si j'enlève toute magie, c'est à dire si la description complète de l'agenda, rendu succincte, est présente dans les appels ce serait plus acceptable ? Genre: ``` agenda = build_virtual_agenda(''' Agenda 1 Bureau monday-wednesday;08:00-12:00,14:00-17:00 thurday-friday:10:00-14:00 Agenda 2 Bureau monday-friday;08:00-12:00,14:00-17:00 ''') # agenda._agenda_1.bureau est défini ``` > 1aaef3ba4cf815ec74 : l'unique test utilise un agenda virtuel (plutôt qu'un agenda simple), vraiment pas idéal pour comprendre, possible d'ajouter des petits tests sur des cas bateaux ? Ok. > Global : les typing.Whatever, j'aimerais qu'on ait une discussion globale à ce sujet qu'on s'accorde ou pas pour introduire ce genre de choses dans Publik, plutôt que voir ça introduit ici ou là par ceux à qui ça plaît. > À part typing.NamedTuple qui fait quelque chose mais est strictement équivalent à collections.namedtuple() en étant plus lisible, les autres c'est purement de la doc, particulièrement pour les unions je trouve ça pratique de pouvoir décrire qu'un truc peut-être ceci ou cela dans des fonctions qui sont faites pour faciliter l'écriture des tests et leur lecture. Ensuite c'est dans les tests, je pense qu'on peut être plus souple là.
ecazenave commented 2023-04-19 12:06:59 +02:00
Owner
Copy Link

"Si j'enlève toute magie, c'est à dire si la description complète de l'agenda, rendu succincte, est présente dans les appels ce serait plus acceptable ? Genre:"

Oui beaucoup mieux.

"À part typing.NamedTuple " : ok.

"Si j'enlève toute magie, c'est à dire si la description complète de l'agenda, rendu succincte, est présente dans les appels ce serait plus acceptable ? Genre:" Oui beaucoup mieux. "À part typing.NamedTuple " : ok.
bdauvergne force-pushed wip/76335-Decoupage-de-get-all-slots-en-de from 1aaef3ba4c to 067574a813 2023-04-19 15:11:02 +02:00 Compare
bdauvergne commented 2023-04-19 15:12:43 +02:00
Author
Owner
Copy Link

Oui beaucoup mieux.

Voilà, plus de magie que de l'explicite. Concernant le code en lui même et pas juste la partie dans les tests ça ne touche à aucun chemin du code actuellement utilisé; si on exclut le déplacement get_all_slots() qui n'a pas été modifiée.

> Oui beaucoup mieux. Voilà, plus de magie que de l'explicite. Concernant le code en lui même et pas juste la partie dans les tests ça ne touche à aucun chemin du code actuellement utilisé; si on exclut le déplacement get_all_slots() qui n'a pas été modifiée.
bdauvergne force-pushed wip/76335-Decoupage-de-get-all-slots-en-de from 067574a813 to c1766854a6 2023-04-21 13:28:48 +02:00 Compare
bdauvergne force-pushed wip/76335-Decoupage-de-get-all-slots-en-de from c1766854a6 to 8748be2b22 2023-05-02 11:09:40 +02:00 Compare
ecazenave approved these changes 2023-05-10 14:19:11 +02:00
chrono/agendas/models.py Outdated
@ -1111,0 +1157,4 @@
It if is booked, report the slot as full.
"""
assert self.kind != 'events', 'get_all_slots() does not work on events agendas'
ecazenave commented 2023-04-18 15:40:20 +02:00
Owner
Copy Link

Assert introduit en douce alors qu'il est question de déplacer la méthode...

Assert introduit en douce alors qu'il est question de déplacer la méthode...
bdauvergne commented 2023-05-22 15:55:12 +02:00
Author
Owner
Copy Link

J'ai retiré le assert et rebasé.

J'ai retiré le assert et rebasé.
ecazenave commented 2023-05-10 14:19:26 +02:00
Owner
Copy Link

Pour la prochaine.

Pour la prochaine.
bdauvergne force-pushed wip/76335-Decoupage-de-get-all-slots-en-de from 8748be2b22 to 85de60141d 2023-05-22 15:53:02 +02:00 Compare
bdauvergne force-pushed wip/76335-Decoupage-de-get-all-slots-en-de from 85de60141d to 1c898427cd 2023-05-22 15:55:00 +02:00 Compare
bdauvergne force-pushed wip/76335-Decoupage-de-get-all-slots-en-de from 1c898427cd to 0ed652c058 2023-05-30 15:36:18 +02:00 Compare
bdauvergne merged commit df6721d580 into main 2023-05-30 22:24:58 +02:00
bdauvergne deleted branch wip/76335-Decoupage-de-get-all-slots-en-de 2023-05-30 22:24:58 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
ecazenave
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/chrono#67
No description provided.
Delete Branch "wip/76335-Decoupage-de-get-all-slots-en-de"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?