Codecov Report

Merging#824 (ff98bed) intomain (427e8df) willdecrease coverage by12.75%.
The diff coverage is23.36%.

❗ Current headff98bed differs from pull request most recent head2029f36. Consider uploading reports for the commit2029f36 to get more accurate results

@@             Coverage Diff             @@##             main     #824       +/-   ##===========================================- Coverage   83.64%   70.89%   -12.75%===========================================  Files          12       16        +4       Lines        2311     2783      +472       Branches      205      208        +3     ===========================================+ Hits         1933     1973       +40- Misses        373      805      +432  Partials        5        5

Any way I can help with this?

I have a more advanced version of this branch staged locally, that wraps all existing Python functionality and some additional features into a Python class. However the challenges I am facing are the following.

I can't seem to be able to find a good SOLID approach to share a single instance of the Python class with all the different parts of the extension. Specifically the Single Responsibility Principle appears to be violated repeatedly with my proposed design.
There is a package called inversify (https://github.com/inversify/InversifyJS) that could help with the dependency inversion and SRP, but I would have to tear apart most of the extension. Using inversify would only make sense if I had the time to rewrite certain parts of the extension that deal with the whole set of Configuration options. Inverting the dependency just for Python but maintaining the linter and formatting dependencies is somewhat strange.

The second part is that all of this Python work feels somewhat redundant given that we could save ourselves a lot of grief just by usingpipx run fortls orpipx run findent. My main concerns withpipx are that:

• it spawns considerably slower than native calls to fortls (or any other binary)
• It uses caching to improve performance but the cache expires after 14 days
• It does not respect virtual environments, sopipx run fortls will installfortls in whichever placepipx installs binaries, even ifpipx is called from within a virtual environment. I am sure there might be some settings that can help with this, but I haven't found them.

I would be fine usingpipx if it weren't for the last point. I suspect users might complain that Modern Fortran installs stuff outside theirvenv.

As you can tell, my mind is not entirely set with the design or even approach that should be followed to solve the Python issues.

Let me know what you think.

Inverting the dependency just for Python but maintaining the linter and formatting dependencies is somewhat strange.

Yeah that seems strange to me as well.

I'm not sure I quite understand the motivation for this PR.
From my perspective, there's the problem of discovering fortls on Windows, because it typically gets installed in a folder that's not on PATH, but is this the case on linux/mac as well?
Can you clarify?

The way I look at it now is there are basically two use cases:

1. we want to make the usage of the extension as seamless as possible,
   which means being able to install, update, detect and run fortls automatically
2. the users should have the option to override this and run fortls from a custom location

Is this correct?

For the user override, do we have any idea on how often people actually use it?
If it were just a very small number, it might not be worth maintaining in the long term if it complicates things too much, but I have no idea how popular it is or how it's used.

I'm not sure I quite understand the motivation for this PR.
From my perspective, there's the problem of discovering fortls on Windows, because it typically gets installed in a folder that's not on PATH, but is this the case on linux/mac as well?
Can you clarify?

The motivation is to be able to interact with Python in a standardised and abstract manner.
Discovery of fortls on Windows is just one of the issues that would be resolved.
There can be similar issues with formatters,fypp preprocessor andfpm.
There is an entire class of user setups that would face auto-discovery issues.
Similarly, there are virtual environments and interactions with thems-python.python extension that have been requested as features.

Manually specifying the paths, although well documented and trivial to do, seems to be a blocker for many users.
To me that is a clear indicator that the extension design of how Python package auto-discovery is implemented needs to be reworked.

Modern Fortran, as an extension that integrates with Python packages needs to:

1. Work with multiple Python and virtual environments
2. Install packages to the selected Python environment
3. Check if a Python package is installed in the environment
4. Get the location of installed Python binaries in the environment
5. Share the selected Python configuration within all parts of the extension that make use of it

This PR aims to address all of the above bullet points.
There are some minor issues, mostly related to points 3. & 4., but nothing that can't be solved with a bit of Python code.
The more pressing issues are 5. & 1. which carry technical difficulties and design considerations.

I see.

Regarding the point:

1. Work with multiple Python and virtual environments

Not with multiple ones at a time, though, right?
What's the idea about how one would select the venv that's supposed to be used?

If your local code isn't too much of a mess, maybe you can push it to a separate branch and I could have a look at it? Maybe I'll have some ideas. (It doesn't have to work, just to give me an idea...)

I have a more advanced version of this branch staged locally, that wraps all existing Python functionality and some additional features into a Python class. However the challenges I am facing are the following.

I can't seem to be able to find a good SOLID approach to share a single instance of the Python class with all the different parts of the extension. Specifically the Single Responsibility Principle appears to be violated repeatedly with my proposed design. There is a package called inversify (https://github.com/inversify/InversifyJS) that could help with the dependency inversion and SRP, but I would have to tear apart most of the extension. Using inversify would only make sense if I had the time to rewrite certain parts of the extension that deal with the whole set of Configuration options. Inverting the dependency just for Python but maintaining the linter and formatting dependencies is somewhat strange.

The second part is that all of this Python work feels somewhat redundant given that we could save ourselves a lot of grief just by usingpipx run fortls orpipx run findent. My main concerns withpipx are that:

• it spawns considerably slower than native calls to fortls (or any other binary)
• It uses caching to improve performance but the cache expires after 14 days
• It does not respect virtual environments, sopipx run fortls will installfortls in whichever placepipx installs binaries, even ifpipx is called from within a virtual environment. I am sure there might be some settings that can help with this, but I haven't found them.

I would be fine usingpipx if it weren't for the last point. I suspect users might complain that Modern Fortran installs stuff outside theirvenv.

As you can tell, my mind is not entirely set with the design or even approach that should be followed to solve the Python issues.

Let me know what you think.

Honestly, I think you could probably abandon this PR. Virtual environments seem to work totally fine as far as I can tell and this is the standard way to setup non-default Python environments. See my comment in#957