Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

Conversation

tokejepsen
Copy link
Member

Brief description

Reviewable ass extractor improvements.

Description

I cant push directly to the OpenPype branch, so going through my personal fork instead. There are a lot of changes because I've merged in latest develop, so to get a better overview of the file changes we need to merge develop.

This is feature parallel to client extractor, with settings ported to OpenPype workflows with objectset and project settings.

Discussion

I don't agree with having an exportSequence attribute because in OpenPype workflows you can define frame range easily, so it makes more sense to just support single frame publishes (which it already does). I propose to remove this attribute to simplify code and workflow.
So to note, current workflow of using current frame to export is prone to user errors and unpredictability.

Testing notes:

  1. Create and publish Ass instance.
  2. Use Arnold's kick executable to validate renders.

jakubjezek001 and others added 30 commits November 18, 2022 13:54
…d-clip-with-colorspace-remapping

Flame: Loading clip with native colorspace resolved from mapping
…se-extension

Webpublisher: extension is lowercased in Setting and in uploaded files
…published_hook

Fix - typo in copy last published hook
…er_with_more_info

Ftrack: Event server status give more information about version locations
…nd-reset-tab

Nuke: loaded nodes set to first tab
…ctPlates-from-Hiero

Hiero: loading effect family to timeline
Co-authored-by: Jakub Trllo <43494761+iLLiCiTiT@users.noreply.github.com>
…next_animation

Publisher: Easy access to publish page from create page
iLLiCiTiT and others added 21 commits December 6, 2022 15:14
…l_source

General: Integrate thumbnail looks for thumbnail to multiple places
…sactions-Handle-cases-when-source-and-destination-is-the-same

File transactions: Source path is destination path
…ltipart_fix

General: Oiio conversion multipart fix
…t-viewer-and-display-regex-fix

Nuke: correct detection of viewer and display
…ed_enhancement

General: Collection Audio speed up
- use lib for attribute context
- remove mask class method
@ynbot
Copy link
Contributor

ynbot commented Dec 6, 2022

Task linked: OP-3924 Implement ASS Extractor

@tokejepsen tokejepsen self-assigned this Dec 6, 2022
Copy link

@hound hound bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Traceback (most recent call last):
Traceback (most recent call last):
  File "/usr/local/bin/flake8", line 8, in 
    sys.exit(main())
  File "/usr/local/lib/python3.8/dist-packages/flake8/main/cli.py", line 18, in main
    app.run(argv)
  File "/usr/local/lib/python3.8/dist-packages/flake8/main/application.py", line 393, in run
    self._run(argv)
  File "/usr/local/lib/python3.8/dist-packages/flake8/main/application.py", line 381, in _run
    self.run_checks()
  File "/usr/local/lib/python3.8/dist-packages/flake8/main/application.py", line 300, in run_checks
    self.file_checker_manager.run()
  File "/usr/local/lib/python3.8/dist-packages/flake8/checker.py", line 331, in run
    self.run_serial()
  File "/usr/local/lib/python3.8/dist-packages/flake8/checker.py", line 315, in run_serial
    checker.run_checks()
  File "/usr/local/lib/python3.8/dist-packages/flake8/checker.py", line 598, in run_checks
    self.run_ast_checks()
  File "/usr/local/lib/python3.8/dist-packages/flake8/checker.py", line 502, in run_ast_checks
    for (line_number, offset, text, check) in runner:
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checker.py", line 56, in run
    parser.visit(self.tree)
  File "/usr/lib/python3.8/ast.py", line 371, in visit
    return visitor(node)
  File "/usr/lib/python3.8/ast.py", line 379, in generic_visit
    self.visit(item)
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checker.py", line 39, in visit_ClassDef
    self.capture_issues_visitor('ClassDef', node)
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checker.py", line 33, in capture_issues_visitor
    self.generic_visit(node)
  File "/usr/lib/python3.8/ast.py", line 381, in generic_visit
    self.visit(value)
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checker.py", line 36, in visit_Call
    self.capture_issues_visitor('Call', node)
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checker.py", line 30, in capture_issues_visitor
    issues = checker.run(node)
  File "/usr/local/lib/python3.8/dist-packages/flake8_django/checkers/render.py", line 22, in run
    if isinstance(arg, ast.Call) and arg.func.id == 'locals':
AttributeError: 'Attribute' object has no attribute 'id'

if not self.legacy_subsets:
return
self.add_convertor_item("Found {} incompatible subset{}.".format(
len(self.legacy_subsets), "s" if len(self.legacy_subsets) > 1 else "")
Copy link

Choose a reason for hiding this comment

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

line too long (82 > 79 characters)

new_item["label"] = new_item.pop("creator_label")
new_item["identifier"] = new_item.pop("creator_identifier")
new_failed_info.append(new_item)
self.add_error_message_dialog(event["title"], new_failed_info, "Creator:")
Copy link

Choose a reason for hiding this comment

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

line too long (82 > 79 characters)


from openpype.pipeline.publish import RepairAction
Copy link

Choose a reason for hiding this comment

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

redefinition of unused 'RepairAction' from line 8

thumbnail_path
]
self.log.debug("thumbnail args:: {}".format(args))
_output = run_subprocess(args)
Copy link

Choose a reason for hiding this comment

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

local variable '_output' is assigned to but never used

mov_path
]
self.log.debug("mov args:: {}".format(args))
output = run_subprocess(args)
self.log.debug(output)
_output = run_subprocess(args)
Copy link

Choose a reason for hiding this comment

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

local variable '_output' is assigned to but never used

try:
convertor.find_instances()

except:
Copy link

Choose a reason for hiding this comment

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

Do not use bare except:, it also catches unexpected events like memory errors, interrupts, system exit, and so on. Prefer except Exception:. If you're sure what you're doing, be explicit and write except BaseException:.
do not use bare 'except'

exc_info = sys.exc_info()
self.log.warning(error_message.format(identifier, exc_info[1]))

except:
Copy link

Choose a reason for hiding this comment

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

Do not use bare except:, it also catches unexpected events like memory errors, interrupts, system exit, and so on. Prefer except Exception:. If you're sure what you're doing, be explicit and write except BaseException:.
do not use bare 'except'

exc_info = sys.exc_info()
self.log.warning(error_message.format(identifier, exc_info[1]))

except:
Copy link

Choose a reason for hiding this comment

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

Do not use bare except:, it also catches unexpected events like memory errors, interrupts, system exit, and so on. Prefer except Exception:. If you're sure what you're doing, be explicit and write except BaseException:.
do not use bare 'except'


@abstractmethod
def _send_instance_changes_to_client(self):
instance_changes = self._get_instance_changes_for_client()
Copy link

Choose a reason for hiding this comment

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

local variable 'instance_changes' is assigned to but never used

self._explicitly_selected_groups.append(group_name)
new_widget.set_selected(True)

def _select_item_extend_to(self, instance_id, group_name, new_widget):
Copy link

Choose a reason for hiding this comment

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

'InstanceCardView._select_item_extend_to' is too complex (36)

@tokejepsen tokejepsen added type: enhancement Enhancements to existing functionality host: Maya labels Dec 6, 2022
@tokejepsen
Copy link
Member Author

It might also be useful to explore with the client whether using renderlayers is a better options than objectsets. The objectsets workflow is a bit like a blackbox, unless you are very confident in using the pipeline. Using renderlayers would mean that you could test the renders in Maya, then it's "just" a matter of implementation to convert renderlayers to ass files.

@antirotor antirotor merged commit 14e6d0f into ynput:feature/OP-3924_implement-ass-extractor Dec 8, 2022
@BigRoy BigRoy mentioned this pull request Apr 5, 2023
65 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
host: Maya type: enhancement Enhancements to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.