Maintenance pre-commit and picture#108
Open
UlysseDurand wants to merge 13 commits into
Open
Conversation
jourdain
reviewed
Apr 13, 2026
jourdain
reviewed
Apr 13, 2026
Contributor
|
@jourdain Based on what you said in your comments, should we also update the cookiecutter? see cookiecutter pre-commit |
Collaborator
|
yes we should |
Ref: https://github.com/Kitware/trame-cookiecutter/commit/ dc80e3a94410b7e9c5f424771670ed6a18a08b7a
jourdain
approved these changes
Apr 16, 2026
Author
|
My mistake last time I didn't update the |
jourdain
reviewed
Apr 20, 2026
jourdain
reviewed
Apr 20, 2026
jourdain
reviewed
Apr 20, 2026
jourdain
reviewed
Apr 20, 2026
jourdain
reviewed
Apr 20, 2026
jourdain
reviewed
Apr 20, 2026
jourdain
reviewed
Apr 20, 2026
jourdain
reviewed
Apr 20, 2026
jourdain
reviewed
Apr 20, 2026
- Remove .flake8 file - Right noqa codes for test_import.py - Revert and ignore static_viewer.html from prettier Ref: Kitware#108 (comment) Ref: Kitware#108 (comment) Ref: Kitware#108 (comment)
UlysseDurand
force-pushed
the
maintenance
branch
from
862158b to
7e873ea
Compare
jourdain
reviewed
Apr 23, 2026
|
|
||
| @export_rpc("viewport.axes.orientation.visibility.update") | ||
| def update_orientation_axes_visibility(self, view_id, show_axis): | ||
| def update_orientation_axes_visibility(self, view_id, _depth): |
Collaborator
There was a problem hiding this comment.
why this change of name?
Author
There was a problem hiding this comment.
My mistake, it comes from a bad "find and replace" I did.
This UlysseDurand@77fa173 puts _show_axis.
jourdain
reviewed
Apr 23, 2026
|
|
||
| @export_rpc("viewport.axes.center.visibility.update") | ||
| def update_center_axes_visibility(self, view_id, show_axis): | ||
| def update_center_axes_visibility(self, view_id, _depth): |
Collaborator
There was a problem hiding this comment.
why about that change of name?
Author
There was a problem hiding this comment.
My mistake, it comes from a bad "find and replace" I did.
This UlysseDurand@77fa173 puts _show_axis.
jourdain
reviewed
Apr 23, 2026
| dataset_id, | ||
| context, | ||
| _depth, | ||
| _requested_fields, |
Collaborator
There was a problem hiding this comment.
keep the original name for kwarg usage.
Author
There was a problem hiding this comment.
If I understood well, in UlysseDurand@b6ebad5, I reverted to requested_fields variable name, and added a noqa for unused variable name
jourdain
reviewed
Apr 23, 2026
| CONVERT_LUT = False | ||
| SKIP_LIGHT = False | ||
|
|
||
| class LUTConfig: |
Collaborator
There was a problem hiding this comment.
Where is that coming from?
Author
There was a problem hiding this comment.
I had these pre-commit errors that indicate that no global should be used
https://docs.astral.sh/ruff/rules/global-statement^[\PLW0603^[\ Using the global statement to update `CONVERT_LUT` is discouraged
--> src/trame_vtk/modules/vtk/serializers/initialize.py:40:12
|
39 | def encode_lut(value=True):
40 | global CONVERT_LUT
| ^^^^^^^^^^^
| CONVERT_LUT = value
|
https://docs.astral.sh/ruff/rules/global-statement^[\PLW0603^[\ Using the global statement to update `SKIP_LIGHT` is discouraged
--> src/trame_vtk/modules/vtk/serializers/initialize.py:45:12
|
44 | def skip_light(value=True):
45 | global SKIP_LIGHT
| ^^^^^^^^^^
46 | SKIP_LIGHT = value
|
https://docs.astral.sh/ruff/rules/global-variable-not-assigned^[\PLW0602^[\ Using global for `CONVERT_LUT` but no assignment is done
--> src/trame_vtk/modules/vtk/serializers/initialize.py:50:12
|
49 | def lookup_table_serializer_selector(*args, **kwargs):
50 | global CONVERT_LUT
| ^^^^^^^^^^^
| if CONVERT_LUT:
52 | return lookup_table_serializer2(*args, **kwargs)
jourdain
reviewed
Apr 23, 2026
jourdain
reviewed
Apr 23, 2026
jourdain
reviewed
Apr 23, 2026
Now it is replaced with `_show_axis` Ref: Kitware#108 (comment) Ref: Kitware#108 (comment)
- Revert variable name for kwarg usage (add noqa ARG001) in imagedata serializer - Handle None data array in mesh serialization - Change import order Ref: Kitware#108 (comment) Ref: Kitware#108 (comment) Ref: Kitware#108 (comment) Ref: Kitware#108 (comment)
Collaborator
|
LGTM, but you need to resolve conflicts. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
As requested in Kitware/trame#842
pre-commit:
Added same pre-commit rules as in the cookiecutter
picture:
Added picture from the documentation
In Kitware/trame#842 the
Web versioningisn't checked when it was added inc9463ef