Modify Funtofem Mphys Imports

OpenMDAO / mphys

Multiphysics with OpenMDAO

Other

47 stars 31 forks source link

Modify Funtofem Mphys Imports #152

Closed sean-engelstad closed 1 year ago

sean-engelstad commented 1 year ago

We are cleaning up funtofem Cython and python files, including moving the funtofem/mphys folder into the pyfuntofem folder, see the PR https://github.com/smdogroup/funtofem/pull/183.
The import of the MeldBuilder class would be changed from from funtofem.mphys import MeldBuilder to from pyfuntofem import MeldBuilder. Another option to import after this change would be from pyfuntofem.interface.mphys import MeldBuilder.

sean-engelstad commented 1 year ago

I made the requested change from @timryanb so that the imports from pyfuntofem will now be from pyfuntofem.mphys import MeldBuilder. I've also updated the imports in this PR appropriately.

kejacobson commented 1 year ago

Can you confirm that the conda package will still be funtofem or do we need to change this to pyfuntofem as well?

Also, you'll need to retroactively verify your commits before we can merge this.

sean-engelstad commented 1 year ago

@kejacobson the conda package for funtofem currently puts the funtofem and pyfuntofem folders into the site-packages/python3.*/ directory of your anaconda envs library. As of right now you could import it with from funtofem.mphys import MeldBuilder. After this change, either by installing through anaconda or the git repo manually, you will need to import with from pyfuntofem.mphys import MeldBuilder. Also, with this change, only the pyfuntofem folder will be put in the conda-python folder of your current environment.

kejacobson commented 1 year ago

I understand the file move and import change, but my question was: is the conda package name going to remain funtofem or are you going to change that to pyfuntofem as well? That is, will this line need to also be pyfuntofem:

mamba install -c "smdogroup/label/complex" -c smdogroup -c conda-forge tacs funtofem -q -y;

sean-engelstad commented 1 year ago

That's a good question. It kind of bothers me that the repo is called funtofem and the python package is pyfuntofem, but I think we'll leave it like that for now. Thus, the conda install command in the unit tests for mphys should remain the same.

sean-engelstad commented 1 year ago

@kejacobson, I talked with @bburke38 and we decided to change the funtofem PR to put all python and cython code into the funtofem folder so that way the python module matches the conda package and repo name. Otherwise it is confusing and could be problematic for new users. As a result, the funtofem.mphys imports will not be changed and this pull request is unnecessary now, so I'm going to close it.