Skip to content

Conversation

@baptistejub
Copy link

Pour garder une trace un peu plus claire des changements amenés par ce fork.

À l'origine une grosse passe d'optimisation du parsing par @jpbougie (https://github.com/demarque/creek/tree/optimize-parse-speed) qui a remplacé le parsing DOM par un parsing "Sax" (classe Reader de Nokogiri qui est une interface de Sax).

Par la suite, présentés ici, quelques ajustements :

  1. Merge d'un fix sur l'interprétation des dates
  2. Mise à jour d'un appel à BigDecimal pour le passage à ruby 2.7
  3. Un autre fix sur l'interprétation des dates : plutôt que Time, on utilise DateTime qui permet d'avoir un temps en UTC directement (Time va utiliser le fuseau du serveur).
  4. Ajout d'une option au parsing des headers pour avoir une représentation des rows sous forme de hash { header => value } (un peu comme CSV qui a aussi une option headers), pour un accès un peu plus facile aux valeurs pour l'appelant.
    4.1. Une possibilité similaire a aussi été ajoutée au repo officiel, mais ici c'est une implémentation maison qui marche avec le travail d'optimisation déjà fait.
    4.2. Au départ une option pensée pour être utilisée dans Tally.
    4.3. Performance : jusqu'à 25 % plus lent que la version "array simple" (testé sur un énorme Excel de 188 Mo : de 700 à 860 secondes); consommation de RAM similaire.
    4.4. Les headers sont parsés en faisant une première lecture partielle du fichier, jusqu'à la ligne des headers. L'accès aux rows reparse le fichier de 0. Je pense que c'est acceptable côté performance.
  5. Un peu plus de documentation dans le README à propos de l'option présentée ci-dessus.

demimismo and others added 3 commits October 29, 2020 14:06
* Always return a Time object for DateTime cells

* Fix specs for DateTime parsing
…ad of array of values)

Fix datetime conversion
@header_row_number = row_number.to_s

rows_with_meta_data.each do |row|
return (@headers = row['cells'].any? && row['cells']) if @header_row_number == row['r']
Copy link
Member

Choose a reason for hiding this comment

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

Hmm cette méthode pourrait donc retourner false? Ça serait à ajuster dans la documentation

baptistejub and others added 2 commits February 3, 2021 20:08
Co-authored-by: Jean-Philippe Bougie <jean-philippe@demarque.com>
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.

4 participants