Fixing multiple : split issue and others small issues
#30
Conversation
This file contains 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
Added a debug log statement to display the split comments for better debugging and understanding of the code flow.
add debug logs, update product names to avoid duplicity. Add used clip product names list for reference.
- Simplified accessing tape_name attribute directly instead of using get_value() method.
- Updated logger instantiation to use Logger.get_logger(__name__) - Replaced self.log with log for consistency and clarity
…level to DEBUG in CreateShotClip - Added debug logging statement for used_names_list in PublishableClip class - Changed the logger setup to use Python's built-in logging module with DEBUG level in CreateShotClip class
- Refactored widget creation method parameters order for clarity. - Updated logger initialization to use custom Logger class.
- Refactored debug log statements for clip_product_name - Updated logging to include track index and clip index when needed
- Added debug logs for clip_product_name, self.base_product_name, and self.track_name to provide more visibility during processing.
- Update how clip product names are generated and handled in the code to improve clarity and consistency.
- Updated variable names for clarity - Improved handling of attribute values based on type - Enhanced regex pattern usage for better matching
Simplified a method call by removing unnecessary line breaks.
iLLiCiTiT
reviewed
Oct 28, 2024
client/ayon_flame/plugins/publish/collect_timeline_instances.py
Outdated
Show resolved
Hide resolved
|
Is this https://github.com/ynput/ayon-flame/pull/30/files#diff-4e3d68a382b1e3c82be1d67b339fd5931b12f5e8ab16ba58d4df5bc08fb6e198R220 the only change in this PR? Because there is a lot of changes that are not related to description, it is really hard to see what "actually" changed... If there would be option to move code refactor to different PR it would help. |
iLLiCiTiT
reviewed
Oct 28, 2024
iLLiCiTiT
reviewed
Oct 28, 2024
Co-authored-by: Jakub Trllo <[email protected]>
Co-authored-by: Jakub Trllo <[email protected]>
- Refactored the loop to handle clip frame range comparisons more efficiently. - Improved handling of product names and indices for track clips.
jakubjezek001
commented
Oct 31, 2024
Comment on lines
250
to
282
| pattern = TXT_PATTERN | ||
| if a_type in ("number", "float"): | ||
| pattern = NUM_PATERN | ||
| pattern = NUM_PATTERN | ||
|
|
||
| res_goup = pattern.findall(value) | ||
| res_group = pattern.findall(value) | ||
|
|
||
| # raise if nothing is found as it is not correctly defined | ||
| if not res_goup: | ||
| if not res_group: | ||
| raise ValueError(( | ||
| "Value for `{}` attribute is not " | ||
| "set correctly: `{}`").format(a_name, split)) | ||
|
|
||
| if "string" in a_type: | ||
| _value = res_goup[0] | ||
| _value = res_group[0] | ||
| if "float" in a_type: | ||
| _value = float(res_goup[0]) | ||
| _value = float(res_group[0]) | ||
| if "number" in a_type: | ||
| _value = int(res_goup[0]) | ||
| _value = int(res_group[0]) | ||
|
|
||
| attributes["xml_overrides"][a_name] = _value | ||
|
|
||
| # condition for resolution in key | ||
| if "resolution" in key.lower(): | ||
| res_goup = NUM_PATERN.findall(value) | ||
| # check if axpect was also defined | ||
| res_group = NUM_PATTERN.findall(value) | ||
| # check if aspect was also defined | ||
| # 1920x1080x1.5 | ||
| aspect = res_goup[2] if len(res_goup) > 2 else 1 | ||
| aspect = res_group[2] if len(res_group) > 2 else 1 | ||
|
|
||
| width = int(res_goup[0]) | ||
| height = int(res_goup[1]) | ||
| width = int(res_group[0]) | ||
| height = int(res_group[1]) | ||
| pixel_ratio = float(aspect) | ||
| attributes["xml_overrides"].update({ | ||
| "width": width, |
There was a problem hiding this comment.
All these changes are related to typo fixing.
jakubjezek001
commented
Oct 31, 2024
Comment on lines
+16
to
+17
| NUM_PATTERN = re.compile(r"([0-9\.]+)") | ||
| TXT_PATTERN = re.compile(r"([a-zA-Z]+)") |
There was a problem hiding this comment.
All these changes are related to typo fixing.
iLLiCiTiT
reviewed
Nov 7, 2024
iLLiCiTiT
reviewed
Nov 7, 2024
- Adjusted condition to check for exact match instead of inclusion.
- Refactored how clip product names are generated and validated to avoid duplicates.
- Adjusted condition for clip inclusion - Added new key formation for clip uniqueness - Updated the list used for duplicity check
…t-not-identifying-hierotracks
…consistency. - Removed unused Logger import - Updated log reference to self.log
iLLiCiTiT
approved these changes
Nov 12, 2024
- Convert width, height, and pixel aspect ratio values to integers or floats as appropriate in multiple files.
robin-ynput
approved these changes
Nov 14, 2024
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.
Changelog Description
Additional info
:in clip comments was breaking our sponsoring clients workflow for AY-6259_Flame: vertical alignment not identifying hieroTracks #14 and AY-6976_Validation shot name limitation #27 this might be fixing those issues too.resolves #14
resolves #21
resolves #27