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

Inconsistency issue #110

Open
koubaa opened this issue Oct 9, 2023 · 6 comments
Open

Inconsistency issue #110

koubaa opened this issue Oct 9, 2023 · 6 comments

Comments

@koubaa
Copy link
Collaborator

koubaa commented Oct 9, 2023

See related PyMechanical issue

For this user, get_mechanical_path(False) poins to v212

ansys.tools.path.get_mechanical_path(False)
'C:\Program Files\ANSYS Inc\v212\aisol\bin\winx64\AnsysWBU.exe'

But,

ansys.mechanical.core.find_mechanical()[0].

apparently points to a newer installation.

@Gryfenfer97
Copy link
Contributor

According to the documentation, these functions do two different things
get_mechanical_path:

Acquires Ansys Mechanical Path from a cached file or user input

find_mechanical:

Search for the Mechanical path in the standard installation location.

@koubaa
Copy link
Collaborator Author

koubaa commented Oct 9, 2023

@Gryfenfer97 @samigithub2022 I see. I think the problem here is that the user did this:

Install and try to use PyMechanical, which runs find_mechanical and then caches the location in appdata. Then, the user gets an error message about requiring 232+, so they install it. But then when they try to use PyMechanical it tries to use the cached version.

I think we most probably have to make sure that trying to use PyMechanical with an older version of Mechanical installed does not cause any artifact to be produced in the user's appdata pointing to that older version.

@Gryfenfer97 on another note - is there a quick API to wipe the config of ansys.tools.path? In the linked bug, I asked the user to use platformdirs directly, but its not so convenient

@Gryfenfer97
Copy link
Contributor

_clear_config_file.
But it is not exposed in the __init__.py so he will have to call it using from ansys.tools.path.path import _clear_config_file.

I think we should expose it. From the docstring:

"""Used by tests. We can consider supporting it on the library"""

@koubaa
Copy link
Collaborator Author

koubaa commented Oct 9, 2023

The takeaway for me is that we should (1) support clear_config_file (imo the name be different, config file is an implementation detail that we shouldn't necessarily expose) and (2) fix the scenario where this might happen in PyMechanical or other applications or (3) add some minimum version argument to things like get_mechanical_path so that it ignores the config file if the cached version is too old, and maybe even adds multiple versions to the config file.

@koubaa
Copy link
Collaborator Author

koubaa commented Oct 19, 2023

The correct way for client libraries is to use "get" first, then use "find" if its not available in "get", but never to save unless the user explicitly does so. We need to make sure that PyMechanical does it that way

@koubaa
Copy link
Collaborator Author

koubaa commented Oct 19, 2023

Actually, get_* doesn't do what it is documented, it also does a find. For dyna at least, we need a flag that says controls whether get can do a find

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

No branches or pull requests

2 participants