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 ability to validate folders of documents to mxvalidate.py #1773

Closed

Conversation

kwokcb
Copy link
Contributor

@kwokcb kwokcb commented Apr 12, 2024

Issue

It is cumbersome to try and validate a set of documents using mxvalidate.

Change

  • Add ability to scan folders for MaterialX documents for mxvalidate.
  • It will also be more efficient as standard libraries will only be loaded once for the set of documents to validate.

@jstone-lucasfilm
Copy link
Member

This is an interesting proposal, @kwokcb, and my main hesitation is that it splits our mxvalidate.py example script into two separate modes (file and folder), and we haven't taken this approach with any of our other examples.

If I ran into a production situation that required batch validation, my initial instinct would be to use mxvalidate.py in a for loop, and of course you can move beyond our example scripts to custom Python code if more sophisticated logic is required.

What are your thoughts?

@kwokcb
Copy link
Contributor Author

kwokcb commented May 28, 2024

Hi @jstone-lucasfilm ,

I think the criteria for a command which handles files / folders vs just files is if there is something that needs to use output option per file. e.g. "bake" could be confusing if you did a folder at a time since there are output options for image folder / names per input file.

Something like mxvalidate or mxformat are easier to use if they can take either a file or folder since all that they do is work 1:1 per file. It would be nice if mxformat could handle just a single file vs always a folder (opposite issue).

@jstone-lucasfilm
Copy link
Member

@kwokcb We've now merged development work on MaterialX 1.39 back to the main branch of MaterialX, in preparation for the final weeks of development on the 1.39.0 release. When you have a chance, could you retarget this pull request back to the main branch as well?

@jstone-lucasfilm
Copy link
Member

@kwokcb I'll close out this pull request for now, since it's framed in terms of a branch that we're planning to delete, but feel free to propose similar changes to the main branch if you're still interested in this idea!

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