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

Option to keep orthoimagery (RGB+NIR) when colorizing a LAS #3

Merged
merged 4 commits into from
Aug 8, 2023

Conversation

CharlesGaydon
Copy link
Collaborator

@CharlesGaydon CharlesGaydon commented Jun 21, 2023

During colorization, there may be two use cases where it is desirable to keep the orthoimages:

  • Quick inspection of LAS content using orthoimages
  • Creation of a mixed Lidar colorized and orthoimage dataset

I propose simply returning the temporary paths used in decomp_and_color, which prevents automatic deletion of temporary files as long as their file is maintained within the execution scope.

No breaking changes, and the usage is as follows:
tmp_ortho, tmp_ortho_irc = decomp_and_color(*args, **kwargs)

Simply using decomp_and_color(*args, **kwargs) still destroys the temporarty files since their reference is out of scope (they are garbage collected)

@CharlesGaydon CharlesGaydon changed the title Add .vscode folder to .gitignore. Option de conserver les orthoimages RVB+Infrarouge lors de la colorisation d'un LAS. Jun 21, 2023
@CharlesGaydon CharlesGaydon changed the title Option de conserver les orthoimages RVB+Infrarouge lors de la colorisation d'un LAS. Option to keep orthoimagery (RGB+NIR) when colorizing a LAS Jun 21, 2023
@CharlesGaydon CharlesGaydon self-assigned this Jun 21, 2023
@CharlesGaydon CharlesGaydon added the enhancement New feature or request label Jun 21, 2023
pdaltools/color.py Outdated Show resolved Hide resolved
pdaltools/color.py Outdated Show resolved Hide resolved
@CharlesGaydon
Copy link
Collaborator Author

Revue prise en compte.
Pas de test unitaire comme les tests actuels se basent sur des données non versionnées.
Je met le lien vers les données de test de pacasam : ce sont deux blocs de 100m x 50m, que j'ai sous-échantillonnés pour qu'ils ne soient pas trop lourd à versionner.

@leavauchier
Copy link
Collaborator

@CharlesGaydon Est-ce que tu pourrais faire un rebase sur dev stp? (maintenant qu'il y a des données de test)

@CharlesGaydon
Copy link
Collaborator Author

  • Rebased sur la branche master.
  • Ajout d'un test pour vérifier que les images sont bien conservées tant que leur référence est dans le scope.

@CharlesGaydon
Copy link
Collaborator Author

@leavauchier @gliegard Prêt à merge à mon avis :)

Copy link
Contributor

@gliegard gliegard left a comment

Choose a reason for hiding this comment

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

ça me dérange un peu que dans tes modifications, il y a aussi le fait de passer un linter sur le code. Je préfèrerai qu'il n'y ait que tes modifications "utiles" aux concepts de fichiers temporaires.

Copy link
Contributor

@gliegard gliegard left a comment

Choose a reason for hiding this comment

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

Il faudra aussi rebase et s'adapter au changelog et à la version qui ont évoluées entre temps

@gliegard
Copy link
Contributor

Bonjour, J'ai ré arrangé les commit de Charles pour avoir

  • 1 commit avec la fonctionnalite
  • 1 commit avec la typo

Et le passage en version 1.2.0 , version mineure, car garder les fichier tmp est une nouvelle fonctionnalité.

Pour moi on peut merger. Je laisse Léa approuver.

@CharlesGaydon
Copy link
Collaborator Author

Merci Guillaume. Pour le linting j'ai un paramétrage non-standard dans mon VS Code (qui correspond à celui de pacasam avec 144 caractères max). Est-ce que vous appliquez black systématiquement sur ign-pdal-tools ? Si oui, avec la longueur de caractère par défaut de black (88) ?

Ca peut être une valeur à indiquer dans pyproject.toml (cf. lien pacasam).

@CharlesGaydon
Copy link
Collaborator Author

@leavauchier J'ai ouvert une issue sur un bug qui occurt durant la colorisation et qui pourrait être corrigé facilement dans cette PR avant de merge : #14

@leavauchier leavauchier merged commit 6245222 into dev Aug 8, 2023
1 check passed
@leavauchier leavauchier deleted the keep-orthoimagery-tmpfiles branch August 8, 2023 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants