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

Default mechanism for finding ESMFMKFILE in esmpy #117

Closed
aulemahal opened this issue Jan 18, 2023 · 3 comments · Fixed by #151
Closed

Default mechanism for finding ESMFMKFILE in esmpy #117

aulemahal opened this issue Jan 18, 2023 · 3 comments · Fixed by #151

Comments

@aulemahal
Copy link
Contributor

Hi!

This issue is a follow up on conda-forge/esmf-feedstock#91. Since esmpy 8.4.0, importing the module requires the ESMFMKFILE environment variable to be set. When using conda, this variable gets set upon activation of the conda environment. This is already good, but sadly it doesn't cover all use cases. Some are detailed in the discussion there. In summary, the activation step is not always performed in some automated setups or when the python kernel of one environment is accessed by an other one.

I was wondering if in addition to looking for an environment variable, esmpy could also search in some "default" locations to avoid failing in the above cases ? I have seen this kind of behaviour in some other packages and it seems to be that on unix platforms it could be easy to set a short list of those locations that could cover most cases. The first esmf.mk file found would be used.

For example, in the issue above, I suggest a solution that infers the .../lib/ folder by looking at where builtin packages are stored.

What do you think?

I could help in implementing this.

@rokuingh
Copy link
Contributor

Hi @aulemahal, long time no see :)

If I understand this correctly, it sounds similar to the old install procedure. As I'm sure you already know, ESMPy used to save the location of the esmf.mk file for use when accessing the esmf shared library. We decided to remove this because it was prone to failure. In some cases a user would update either their ESMPy code or ESMF installation independently, resulting in some very strange undefined behaviour.

With 8.4.0 we made ESMFMKFILE a required environment variable pointing to a specific esmf.mk file to avoid mismatches in ESMF shared library. There is also now a function to check the version of the ESMF shared library against ESMPy. This will issue a warning if using a beta snapshot (i.e. git version not matching a release tag but sharing the first three digits), or error out if there is a mismatch in the first three digits of the version number.

That said, I believe this version check was implemented after switching away from the old esmf.mk behaviour. The old esmf.mk strategy needed to be updated regardless, but I do see the benefit of combining a strategy like you suggest with the version check. I would welcome this contribution, although I dont have much time for development due to reduced hours while on parental leave. In the event that you were to lead this effort i would be happy to support any resulting pr by completing the testing required for a merge to the develop branch.

Please keep me in the loop, and tag me here or email if needed. Thanks for the suggestion, and happy coding!

@RondeauG
Copy link

RondeauG commented Mar 6, 2023

I think I have a direct use case of what @aulemahal is talking about. I use Pycharm 2022 Community Edition to write my code and while I can point to an existing conda environment, I think Pycharm doesn't activate it in a way that creates environnement variables (in 'Debug' mode, at least).

The net result is that I can't import ESMF 8.4.0 at all with Pycharm, while it worked fine with 8.2.0.

@billsacks
Copy link
Member

I believe this was resolved with #151 and should have been closed when that was merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants