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

feat: No class average if it is not known #355

Closed

Conversation

Bulgus
Copy link
Contributor

@Bulgus Bulgus commented Nov 13, 2024

Checklist d'avant pull request

Veuillez cocher toutes les cases applicables en remplaçant [ ] par [x].

  • Vous avez testé de build le projet avec vos modifications et ce build a réussi
  • Vous respectez les conventions de codage et de nommage du projet
  • Vous utilisez la tabulation pour l'indentation afin de maintenir un code lisible
  • Cette pull request n'est pas un duplicata d'une autre
  • Cette pull request est prête à être revue (review) et fusionnée (merge)
  • Il n'y a pas de TODO (aka des annotations pour du code manquant) dans vos modifications
  • Il n'y a pas d'erreurs de langue dans votre code (grammaire, vocabulaire, conjugaison, orthographe)
  • Les détails des changements ont été décrits ci-dessous
  • Cette pull-request n'est pas une "breaking-change" (des modifications qui vont entraîner la modification du fonctionnement de certaines fonctionnalités déjà existantes)

Changelogs proposés

Si Papillon n'arrive pas à récupérer la moyenne de classe, celle ci est égale à 0.

  • La moyenne de classe est désormais affichée comme "Inconnue" si elle n'est pas récupérable par Papillon (= 0).
  • Il n'est pas non plus possible d'afficher les détails de celle ci en cliquant sur le graphique (Moyenne minimum et maximum, qui ne sont pas connues même si affichées).

Informations supplémentaires

Avant

Moyenne.disponible.MP4
Pas.de.moyenne.disponible.MP4

@Bulgus Bulgus changed the title feta: No class average if it is not known feat: No class average if it is not known Nov 13, 2024
@Kgeek33
Copy link
Contributor

Kgeek33 commented Nov 13, 2024

c'est dans quel cas que Papillon n'arrive pas à récupérer la moyenne de la classe ?
Car normalement, Papillon ne récupère pas la moyenne exacte, mais la calcule en fonction de tes matières (en lien avec l'issue : #356 )

Copy link
Contributor

@ecnivtwelve ecnivtwelve left a comment

Choose a reason for hiding this comment

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

Même question : Papillon devrait toujours calculer approximativement une moyenne si elle n'arrive pas a récupérer la valeur exacte, donc quel est le cas d'usage ?

@Bulgus
Copy link
Contributor Author

Bulgus commented Nov 14, 2024

Merci des infos.
J'ai regardé l'issue, et du coup :
Comment la moyenne est calculée ? C'est basé sur toutes les moyennes de matières les plus hautes / basses ?

Si oui, je pense qu'il vaut mieux ne pas l'afficher, car des moyennes à 19,5 c'est très peu probable... donc pour moi si pas de moyenne retournée par Pronote (c'est le cas dans pas mal d'établissements dont le miens), ne pas l'afficher. ("Inconnue" et graph non cliquable)

Je peux tenter de regarder si je peux faire ça si ça vous semble bon..?

Et pour répondre à la première question, il semble qu'avec mon Pronote, Papillon échoue à trouver une moyenne générale de classe, et la met à 0. Et pensant que cette valeur était retournée lorsque que Pawnote ne pouvait pas récupérer la moyenne, j'ai fais cette PR.

Dites moi ce que vous en pensez, mais en tout cas pour moi pas de moyenne sur Pronote = pas de moyenne de classe calculée

Ou alors que la moyenne générale, et pas la moyenne plus haute / basse car celles ci sont vraiment improbables et à mon avis peu utiles.

@Kgeek33
Copy link
Contributor

Kgeek33 commented Nov 14, 2024

Alors :

  • Je pense que tu devrais désactiver dans le cas général désactivé l'affichage des meilleures et pires moyennes. C'est qqch de calculé et donc improbable
  • Au niveau de ton problème, je pense que c'est un bug en rapport avec l'issue et ton problème. En effet, quand je lis tous vos comportements, on dirait que Pawnote récupère la moyenne quand elle n'est pas disponible (du coup, une moyenne simulée quand disponible). A voir dans le code

Mets tes modifs en commentaire et analyse le 2ème point que j'ai écrit et dis-moi quand tu as des nouvelles ;)

@Kgeek33
Copy link
Contributor

Kgeek33 commented Nov 14, 2024

Et tiens, j'ai pensé à une Feature. Quand c'est une moyenne de classe simulée, afficher une icône, ou qqch pour qu'on comprenne que ce n'est pas la vraie moyenne de classe
Comme sur la v6

@Bulgus
Copy link
Contributor Author

Bulgus commented Nov 14, 2024

@Kgeek33

@Kgeek33
Copy link
Contributor

Kgeek33 commented Nov 14, 2024

Mdrr t'aurait pu faire tout ça dans une seule pr mais Vince t'a devancé en faisant plusieurs pr...

Bon ben tout ce que tu peux faire, c'est ajouter une icône pour la moyenne de la classe (et encore faut que la pr sur la moyenne de la classe soit accepté)

@Kgeek33
Copy link
Contributor

Kgeek33 commented Nov 14, 2024

Si tu veux une occupation pour garder cette pr essayé d'intégrer comme sur la v6 une simulation de notes
Si tu n'as pas connu, c'est un bouton qui te permettait de saisir une note dans une matière, le coeff...
Et le graphique affichait la nouvelle moyenne générale simulée du coup

@Bulgus
Copy link
Contributor Author

Bulgus commented Nov 14, 2024

Oui mdrr

Yep... mais je peux close celle là puisqu'à priori elle ne sera jamais inconnue la moyenne..
Et faut voir pour supprimer la moyenne max et minimum
C'est qui qui décide de ça ?..

@Kgeek33
Copy link
Contributor

Kgeek33 commented Nov 14, 2024

Oui du coup, ta pr ne sert plus.
Décider sur quoi ?

@Bulgus
Copy link
Contributor Author

Bulgus commented Nov 14, 2024

Qui décide de si on garde la moyenne max et minimum ?
Je peux refaire une PR en supprimant la moyenne max et minimum, et en ajoutant un icon si la moyenne est calculée

@Kgeek33
Copy link
Contributor

Kgeek33 commented Nov 14, 2024

Ben en fait, c'est une fonctionnalité qui date (j'pense avant même la v6), donc c'est personne qui décide en soi, juste personne n'a eu l'idée de la remettre en question (perso, j'pense que ça sert à rien mais bon)

Moi, je te conseille de refaire une pr quand toutes les pr de Vince seront merged et aussi de le pas supprimer l'affiche des moyennes bonnes/pires, mais les désactiver (comme t'as fait sur ta pr, rien ne se passe quand on clique sur ta pr)

@Bulgus
Copy link
Contributor Author

Bulgus commented Nov 14, 2024

Okay... donc faut que quelqu'un la remette en question alors...

Nickel merci. Mais pourquoi masquer ?.. Ça fait du code en trop dont on ne se sert pas dans ces cas là nan ?..

@Kgeek33
Copy link
Contributor

Kgeek33 commented Nov 14, 2024

Okay... donc faut que quelqu'un la remette en question alors...

Nickel merci. Mais pourquoi masquer ?.. Ça fait du code en trop dont on ne se sert pas dans ces cas là nan ?..

Oui, tu sais déjà que je suis contre 😁
(et surtout, entre nous, quelle serait la probabilité que les gens sachent qu'il faut cliquer sur le graphique pour afficher des moyennes bonnes/pires ?). C'est tellement mal indiqué !

Ah ça c'est à toi de décider sur ta future pr ! Mais t'as raison, du code en moins et plus d'efficacité :)

@Bulgus
Copy link
Contributor Author

Bulgus commented Nov 14, 2024

Oui 😁
(Oui, c'est vrai que je le savais, mais encore faut-il le savoir.. je vais faire une PR en supprimant tout, je verrais bien, mais si c'est pas merge, faudra me dire à quoi ça sert de laisser..)

Yep :)

@Kgeek33
Copy link
Contributor

Kgeek33 commented Nov 14, 2024

Oui ben tkt pas sur le merged, regarde ma pr #325, c'est quand même pas rien de résoudre tous les bugs mais depuis que j'ai créé ma pr, 10 ont été merged entre temps............

@Kgeek33
Copy link
Contributor

Kgeek33 commented Nov 14, 2024

Si on voit mon msg, j'espère que les façons de review les pr vont changer...

@Bulgus
Copy link
Contributor Author

Bulgus commented Nov 14, 2024

Oui j'ai vu ça... ça fait 2 semaines.. et ça devrait être merge en priorité pour continuer sur un code clean (si je dis pas de bêtise ?)...

@Kgeek33
Copy link
Contributor

Kgeek33 commented Nov 14, 2024

Oui exactement, un code clean et une application stable mais bon...
Par contre, a partir de demain, si tjrs pas de review, je vais commencer à spammer car ça commence à être énervant de prioriser certaines prs qui sont (beaucoup) moins importantes que d'autres (c'est pas pour me mettre à l'avant, loin de là mais comme exemple bien parfait : #281)

@Bulgus
Copy link
Contributor Author

Bulgus commented Nov 14, 2024

Wow 9 octobres... plus d'un mois...
Ouais.. sachant que certaines de mes PR bien moins importantes et moins bien faites sont passées en 1 semaine...

@Kgeek33
Copy link
Contributor

Kgeek33 commented Nov 14, 2024

Allez, on va rien dire 🙄
J'espère que notre workflow (que je travaille avec @Louis-htmlcss ) permettra de faire changer les choses
Si ça t'intéresse, voici le repo (une pr sera tjrs ouverte pour tester) : https://github.com/Louis-htmlcss/test-du-workflows

@Bulgus
Copy link
Contributor Author

Bulgus commented Nov 14, 2024

Expo go intégré dans chaque PR ?? C'est génial..
Oui j'espère que ça aidera...

@Kgeek33
Copy link
Contributor

Kgeek33 commented Nov 14, 2024

Et oui !
#364

@Louis-htmlcss
Copy link
Contributor

Expo go intégré dans chaque PR ?? C'est génial..
Oui j'espère que ça aidera...

Oui j'ai fait ma pr si tu veux la voir ♥️

@Bulgus
Copy link
Contributor Author

Bulgus commented Nov 15, 2024

J'ai vu ça.. et ça peut être très très pratique (voire indispensable pour tester et rapidement review donc...)
GG !

@Kgeek33
Copy link
Contributor

Kgeek33 commented Nov 15, 2024

Mdrr faut closed la pr @Bulgus
Et la refaire plus tard

@Bulgus Bulgus closed this Nov 16, 2024
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.

5 participants