Feature/vectorset : nouvelle implémentation suite à la point review datant du 22-09-2025 avec Théo #116
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…Théo => implémenter les attributs, constructeurs, propriétés et méthodes précisées comme dans le ticket
…ples de fichiers vecteurs
…e fichiers d'entrée vecteur contenus dans le dossier "tests/fixtures"
… à revoir dans le PR du 22-09-2025
…nt à revoir car de nouvelles fonctions ont été rajoutées suite à la revue de code PR du 22-09-2025 par Théo
… globalité suite ajout de nouvelles fonctions
…s en entrée du programme avec "ogr.UseExceptions()" à la ligne 42
… la fonction serializable + revue de from_file comme dans test.py
…% suite revue du code de 'vector.py'
…r_calls_put_data_str()' et 'test_vector_from_file_raises_storageerror_on_none_datasource()'
…ises_from_formaterror_on_jsondecodeerror()' + tester que la sortie de la property de srs est bien une liste
…es méthodes de test
…prime à la fin du test
…ression import module 'sys'
…es tests passent avec une couverture globale à 94%
… est sérializable
…feature/vectorset
…tion de tests manquants
- passage du CHANGELOG au format keepachangelog - déplacement des fichiers de tests dans les dossiers fixtures - migration de fix-encoding-pragma vers pyupgrade dans le pre commit
Dolite
force-pushed
the
feature/vectorset
branch
from
d2fac70 to
13e09b9
Compare
Dolite
reviewed
Jan 5, 2026
Member
Dolite
left a comment
Dolite
left a comment
There was a problem hiding this comment.
Retours généraux :
- La documentation doit être faite en docstring google pour être homogène avec les autres modules
- Les autres modules ne loggent rien, pour laisser aux projets appelant la main sur la quantité d'information à afficher. Il ne faut pas faire de print
Member
There was a problem hiding this comment.
Retours généraux :
- La documentation doit être faite en docstring google pour être homogène avec les autres modules
- Il faudrait documenter les exceptions possibles dans les fonctions, au moins les plus importantes
- Les autres modules ne loggent rien, pour laisser aux projets appelant la main sur la quantité d'information à afficher. Il ne faut pas faire de print.
- L'initialisation des attributs peut se faire directement dans le init, sans passer par des attributs de classe (__path et __tables dans Vector par exemple)
- Si tu veux mettre des exemples d'usage d'une fonction, mets le dans la doc de cette fonction, dans une section
Example(comme dans list_generator de la classe Pyramid par exemple)
| else: | ||
| dataSource = ogr.Open(get_osgeo_path(path), 0) | ||
| if datasource is None: | ||
| raise StorageError("FILE", f"Cannot open vector file/object {working_path}") |
Member
There was a problem hiding this comment.
Mieux vaut mettre dans l'erreur le chemin d'origine, pas celui osgeo (sinon l'utlisateurice risque de ne pas comprendre d'où vient cette valeur qui n'est pas celle de son côté)
src/rok4/vector.py
Outdated
|
|
||
| layer = datasource.GetLayer(i) | ||
| name = layer.GetName() | ||
| count = layer.GetFeatureCount() |
Member
There was a problem hiding this comment.
Ne pas forcer le calcul du nombre de features pour éviter des délais trop long sur les gros fichiers (mettre le premier paramètre à 0 a priori)
| path (str): path to the file/object | ||
| bbox (Tuple[float, float, float, float]): bounding rectange in the data projection | ||
| layers (List[Tuple[str, int, List[Tuple[str, str]]]]) : Vector layers with their name, their number of objects and their attributes | ||
| def from_parameters(cls, path: str, tables: Dict[str, "Table"]) -> "Vector": |
Member
There was a problem hiding this comment.
Le paramètre tables est un simple dictionnaire, chargé depuis le descripteur JSON. Il faut faire dans cette fonction la boucle d'appel au constructeur Table pour créer les instances à partir des informations dans le dictionnaire.
Author
There was a problem hiding this comment.
making a loop for calling the Table constructor has been done !
src/rok4/vector.py
Outdated
| print(self.tables) | ||
| # for each table in the vector data, we get its serializable version | ||
| for table in self.tables: | ||
| serialization["tables"].append(Table.serializable.fget(self.tables[table])) |
Member
There was a problem hiding this comment.
serializable doit être une priopriété de l'instance Table (pas de fget du coup)
src/rok4/vector.py
Outdated
| return serialization | ||
|
|
||
|
|
||
| if __name__ == "__main__": |
…rint + add raises exceptions for the most important functions + initialize attributes directly in the __init__ constructor + serializable is a property Table instance + make in function from_parameters() the call loop to Table constructor to create the instances from data into the dictionary
…feature/vectorset
…feature/vectorset
…arameters()' function
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
headers, docstring et commentaires rédigés tout en anglais + couverture globale des tests unitaires et d'intégration à 94%