search

mgear-dev / mgear_dist

mGear v.3.x.x distribution repository
http://www.mgear-framework.com/
MIT License
184 stars 53 forks source link

Missing vendors #32

Closed tokejepsen closed 5 years ago

tokejepsen commented 5 years ago

Firstly I know what when you build this project, Qt.py and qjsonmodel.py gets copied from the submodules to the correct folder.

I was wondering why not just copy only the files needed instead of having the submodules?

An advantage of doing this, is that a developer can clone the repository setup PYTHONPATH and MAYA_PLUG_IN_PATH and start working the on the python code without having to build the project.

miquelcampos commented 5 years ago

Hi @tokejepsen

The idea was to keep the submodule pointing to the original repo in order to update easily. Not only for this but also cvwrap, math_nodes or other future vendored modules

To be honest, not sure about this. I would like to know @jdrese opinion.

tokejepsen commented 5 years ago

I should also add that with the current code you cannot just setup the PYTHONPATH to point to the submodules like Qt.py and QJsonModel. The issue is that you are expecting those vendors to be in https://github.com/mgear-dev/mgear_dist/tree/master/framework/scripts/mgear/vendor because of these lines:

https://github.com/mgear-dev/mgear_core/blob/5cd608fc6ec7f9c727891d9f35646d7f2df23d31/scripts/mgear/core/pyqt.py#L11 https://github.com/mgear-dev/mgear_core/blob/b1573d72f2f873ffeee27144a798d5312db6937b/scripts/mgear/core/widgets.py#L3

So there is no way of running the project without building it first which makes python development tedious.

miquelcampos commented 5 years ago

I see :O Thanks for the explanation and pointing this issue.

Indeed that is not good.

Let's wait a little for @jdrese comment. But yep I see your point now :) Thanks again!

jdrese commented 5 years ago

Hello guys

I would like to keep the repo submodule thing again because it easier to keep track of updates with Github.

That been said, it is quite true that external libraries been shipped with mGear inside the vendor package might not be the best idea to be inside mgear's namespace.

If you take a Python lib installations, pip will always install whatever dependency lib is needed as an independent install so that each library stays clean and can be removed/updated.

What this implies is that if we deploy mGear trying to keep that (pip) logic it means that we need to warn users that when mGear is loaded (in the pythonpath) they might have some glitches with the Qt.py python package because python will pick the last Qt.py package in the PYTHONPATH. This means that if the user installed version Qt.py differs from the one shipped with mGear then we might get some issues here and there on mgear but most of all they might encounter that on other tools as well and that is not acceptable without warning the user with notice.

What I can suggest is the following.

Using a try and except import statement so that we can have both. When importing mgear.vendor.Qt fails it uses import Qt. What this means is that the user can either use the one shipped with mGear or a local one. For us this means that if you don't want to do a release to work you can add into your Python path the QT submodule repo and it will pick it up all the same. And when it's deployed for users if someone do not want to use our shipped one because it's not the same version as their local one they can simply rename the python file or delete it...

It doesn't seem like the most "beautiful" solution but mGear is not a Python Library, it's a Maya tool so it's not logic that we try hard to follow PIP install logic either. Normally in big software development ALL DEPENDENCIES are always included in the software and not as external installs. So for me the fact that mGear's ships Qt.py is logic (even if it's not a big software). That fact that we are able to work with the shipped one or a local one then makes a lot of sens.

Sorry this was long...

jdrese commented 5 years ago

Forgot to mention that if you do use the python files inside the framework folder we need to add an ignore pyc files to scons when building mgear just so that isn't copied out.

tokejepsen commented 5 years ago

@jdrese that all sounds fair enough.

My solution is to have Qt.py and qjsonmodel.py in the vendor directory. These modules do nothing but redirect to potentially native modules found on PYTHONPATH.

The question is whether Scons can delete the files before pulling from the submodules for distribution?

miquelcampos commented 5 years ago

@tokejepsen yep, I think Scons can delete it.

tokejepsen commented 5 years ago

@tokejepsen yep, I think Scons can delete it.

Does Scons overwrite or do you need to delete it first? I might need some help setting that up, because I'm yet to have build the project.

miquelcampos commented 5 years ago

@tokejepsen I need to check about overwrite. But since you can execute any python code. I think delete is possible using os.remove(). But maybe is worth to try to overwrite first since is a simple solution

miquelcampos commented 5 years ago

@tokejepsen @jdrese

Hello, I have been testing and is possible to get a default QT.py and other vendor modules in the vendor folder, but at release time with excons just get the latest version from the submodules

I am going to close this pull request and create a new issue. For the modules on the vendor folder of mgear_dist, I have added the following comment.

# mGear Dev version.
# this module is not sync with the latest version

So is clear that is what you get when using scons for a release. Just in case.