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

add support for kwargs in instance init #372

Merged
merged 3 commits into from
Oct 19, 2021

Conversation

hannesdelbeke
Copy link
Contributor

@hannesdelbeke hannesdelbeke commented Oct 7, 2021

now instance() and context.create_instance are interchangable, before it would error out since only some kwars were shared.

see forum post explaining it https://forums.pyblish.com/t/instance-vs-create-instance/658

now instance() and context.create_instance are interchangable, before it would error out since only some kwars were shared.
@hannesdelbeke
Copy link
Contributor Author

just discovered the dev docs so PR likely will need some work
https://pyblish.gitbooks.io/developer-guide/content/implementing_a_feature.html

@mottosso
Copy link
Member

Thanks @hannesdelbeke! Yes, the main thing missing here are tests. Something to make sure it works the way you expect, and something to make sure it breaks when you expect it to. The fact that all prior tests pass is great, it means we haven't broken anything anyone depends on.

Other than that, I'm onboard with this change; normally I'd never actually use the Instance constructor directly, and would always go through create_instance but it doesn't hurt having it accessible as well.

@hannesdelbeke
Copy link
Contributor Author

cheers, will look into when I have time.
Instance is great to use directly when not making context plugins. ex child plugins, parented to other plugins

@hannesdelbeke
Copy link
Contributor Author

hannesdelbeke commented Oct 14, 2021

I'd never actually use the Instance constructor directly, and would always go through create_instance

probably because curently pyblish is context heavy, and parent support is still early days.
I believe QML does not show them yet, and from reading another thread the underlying infrastructure might be not in place yet.
see https://forums.pyblish.com/t/family-hierarchy-for-complex-scenes/443

@mottosso
Copy link
Member

That's true, there's a place for hierarchical instances but so far nobody has taken on the challenge. If there isn't yet an issue about this, now would be good time to open one so we can sort out the details. Such as, should the next parent continue if the child of a prior parent fails?

@hannesdelbeke
Copy link
Contributor Author

hannesdelbeke commented Oct 14, 2021

i'll probably follow up on that in an issue when i have time since it aligns with my goals for pyblish.
regarding failure and flow, ideally the user can set this based on their prefered workflow.
sometimes you want to know all the issues in your scene, sometimes you just want to know as quick as possible if there are no issues. really depends on the task you are working on.

@hannesdelbeke
Copy link
Contributor Author

i've added a test testing the new setup.
this test would fail in the old code.
are we good to merge this?

@mottosso mottosso merged commit 4bfd6e1 into pyblish:master Oct 19, 2021
@mottosso
Copy link
Member

Success! Great work @hannesdelbeke. :D

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.

2 participants