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

Maya: Refactor Maya API #1306

Open
antirotor opened this issue Apr 12, 2021 · 12 comments
Open

Maya: Refactor Maya API #1306

antirotor opened this issue Apr 12, 2021 · 12 comments
Assignees
Labels
host: Maya type: bug Something isn't working

Comments

@antirotor
Copy link
Member

antirotor commented Apr 12, 2021

Problem

Maya integration is complex. Main problem with it is that right now most of the code is in one file hosts/maya/api/lib.py. This file is ~2700 lines of code long and including function from looks management to generic dialog popup functions. This should be split into module lib/__init__.py with different packages grouping specific functionality. This way it will be more maintainable and testable.

[cuID:OP-1186]

@antirotor antirotor added type: bug Something isn't working host: Maya MEDIUM labels Apr 12, 2021
@mkolar mkolar added the backend label May 10, 2021
@antirotor
Copy link
Member Author

I would add to this that during that refactor, we should cleanup legacy code and switch to better handling of render setup. It seems we are hitting legacy api limits.

@tokejepsen
Copy link
Member

To make it faster to collect render layers, maybe look at not relaying on switching layers and query the attributes instead; https://gist.github.com/davidlatwe/e33b942967f18f4b61f085e3effe86c7#file-rendersetup_util-py-L67

@mkolar mkolar added cu and removed cu labels Aug 24, 2021
@mkolar mkolar removed the backend label Mar 15, 2022
@BigRoy
Copy link
Collaborator

BigRoy commented Jan 6, 2023

Has this been resolved since? :) Or are there still major pain points to be refactored? If so, it would be good to have a Task list here of bullet points to address.

@BigRoy
Copy link
Collaborator

BigRoy commented Mar 11, 2023

@iLLiCiTiT getting back to my previous comment

Has this been resolved since? :) Or are there still major pain points to be refactored? If so, it would be good to have a Task list here of bullet points to address.

Should we close this?

@iLLiCiTiT
Copy link
Member

This is on @antirotor :)

@BigRoy
Copy link
Collaborator

BigRoy commented Mar 26, 2023

@antirotor please confirm what you'd like to do with this.

@antirotor
Copy link
Member Author

My idea was to split it into more files to make it more managable even if they will at the end link back to the lib. The file is huge with mixed functions to work with attributes, alembic, transforms, menu, colorspaces, xgen, render layers and even though the functions there are mostly documented with docstrings, it is difficult to add/use something from there without spending time deciphering what's there. And it would be easier to add unit and functional tests.

@tokejepsen
Copy link
Member

My main issue with the lib is that unless you know what is available, you end up writing duplicate code while developing, then trying to refactor to use the lib code. Not sure whether that is a valid way to work, but would assume being able to utilize the lib while developing would be faster than refactor afterwards.
Have not got a solution though to how you would introduce knowledge to newcomers about what it available in the lib.

@antirotor
Copy link
Member Author

My main issue with the lib is that unless you know what is available, you end up writing duplicate code while developing, then trying to refactor to use the lib code. Not sure whether that is a valid way to work, but would assume being able to utilize the lib while developing would be faster than refactor afterwards. Have not got a solution though to how you would introduce knowledge to newcomers about what it available in the lib.

That is actually why I've created this issue. I think that splitting it into clearly defined pieces will help a bit - for example: you need to use function that will query something in render layers, first you look into lib_renderlayer.py (or whatever) and if it is not there, you know you need to write something new. No need to crawl over one huge file. I understand that it might add slight overhead from the other side - but it is much easier to refer to one file that bits and pieces scattered over one huge,

@BigRoy
Copy link
Collaborator

BigRoy commented Mar 27, 2023

That is actually why I've created this issue. I think that splitting it into clearly defined pieces will help a bit - for example: you need to use function that will query something in render layers, first you look into lib_renderlayer.py (or whatever) and if it is not there, you know you need to write something new.

I actually don't think we'll get benefit in that at all. For what it's worth I've maybe had a 5% success rate going into openpype/lib and knowing which file I should be looking in.

  • We'll just end up getting the issue that this namespaced commands exists in setdress.py but at the same time in lib.py (or another). I've no idea what to expect in a setdress.py but the majority of functions in there to me don't scream "setdress".
  • There's also render_setup_tools, lib_rendersetup and other lib_render** which from wording sound like they have so much overlap that I personally constantly forget which one I'd need. :) Maybe it helps if each of the files would at least have a top docstring of what it's about and what it's for?
  • Or what about having some empty file
  • Or maybe have a maya/api/shader_definition_editor.py in the maya/api which is actually a Tool but the command to start it is in maya/api/commands.py - weird maybe?
  • There's also this SHAPE_ATTRS definition which is unused but appears to be the same here and here

I think if we end up separating to files we better start defining what ends up in which file. I can definitely see value in separating out clear pieces related to a specific functionality, for example this "lookdev" region in lib.py can be moved into a separate file, but then what's the name lib_looks.py, lib_lookdev.py, lib_shaders.py? Or is it just look.py?

There's definitely worth in cleaning this up, but I don't expect it'll provide that much clarity on knowing what's there in the codebase without doing a small hunt yourself unless there's documentation for a dev to go through.

@antirotor
Copy link
Member Author

That's because now it is all over the place. I think the clearest and most "pythonic" way would be to move things related to specific workflows to separate files - like what you've mentioned with looks. looks.py is pretty obvious. The same with attributes.py, render_setup.py and so on. We'll need to keep lib for the generic functions but anything that is used exclusively for specific type of workflow should be imho in separate file. And yes, ideally with file-level docstring.

@BigRoy
Copy link
Collaborator

BigRoy commented Mar 27, 2023

Let's give it a go. Anyone up for taking this on? Who eats refactors for breakfast?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
host: Maya type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants