Python Packaging

[gswifort]

Originally posted by @gswifort in https://github.com/CEXT-Dan/PyRx/discussions/67#discussioncomment-9687805

[gswifort]

I have taken preliminary steps to pack and distribute the package. https://github.com/gswifort/PyRx/tree/73-cad-bundle-installer

1. I created a main package called pyrx,
2. I moved the stub files for PyAp, PyDb, PyEd etc. to pyrx/PyRxStubs. It was necessary to update the imports, for this I also updated genDoc.py,
3. I moved the ActiveX files to the pyrx/activex directory,
4. I moved the PyRxDebug.py file to pyrx/debug.py - I also improved it a bit,
5. I added the pyrx/PyRxBin directory with .*rx files, in the future we plan to copy them (on python -m pyrx bundle --install) to the global directory (%localappdata%\Programs\PyRx\Bin\), according to #73,
6. I added the files requirements.txt and requirements-dev.txt,
7. I updated the .gitignore file, many entries are probably redundant there, but I used automatic generation,
8. I added the setup.py file (please correct/complete the data) and MANIFEST.in,
9. In the setup.py file I assumed that in the basic version wxpython is enough, while pywin32 is optional (pip install pyrx[activex]), I did not add debugpy at all, when trying to import import pyrx.debug will display an error with the message Optional dependency not installed. Install debugpy (python -m pip install debugpy),
10. The package version will be read from the file pyrx/__init__.py, I assumed that the package version will be the same as the PyRx*.*rx version.

All you need to do to build the package is activate the virtual environment and run the build.bat file (install the missing packages first (pip install -r requirements-dev.txt). Ready packages will be placed in the dist folder.

A few things to discuss:

• [ ] I suggest using isort and black (also add to the guidelines),

• [ ] pyrx_onload.py - we already discussed this in #1, but I see room for improvement here I assume that files from the Bin directory will/can be installed globally, but CAD will be run from different (virtual) environments. Ideally, the pyrx_onload.py file would be loaded based on the environment settings, like so:

  • first check the PYRX_ONLOAD environment variable, which will point to the file to be loaded,
  • the above option can be checked for two scenarios - a path (relative or absolute) or a python module in the format pkg.subpkg.module is provided,
  • if the environment variable is not set, we check whether the file pyrx_onload.py exists in the working directory (os.getcwd()),
  • if it does not exist, look for it in the directory where the loaded file RxLoader*.*rx is located,
  • as a last resort, we search in the global directory (%localappdata%\Programs\PyRx\Bin\),
  • you can also abandon the search after detecting the environment variable PYRX_ONLOAD=False/false/0.

[CEXT-Dan]

Wow, you’ve done a lot of amazing work!

I had started on guidelines, but it’s for the C++ stuff, not sure I’m qualified to make guidelines for proper Python lol. Honestly, I started reading PEP8, and I stopped when I got to the underscores, and 79 characters, really? I understand this is a requirement for some companies and Python users will absolutely die on that hill. This project should probably comply... mostly

The only real requirements I have are. Libraries- The must have some test coverage, must not hold on to references longer than necessary, or prevent objects from being garbage collected in a timely manner.

Black is a linter, right? I had installed super-linter and it kicked out all the autogenerated ActiveX stuff. Clang kicked out all the C++ boost::python stuff, which we need. I disabled it for now. I don’t have a problem enabling one for Python, if there’s a way to set it to ignore certain files.

Regarding isort, this changes the order of the imports, correct? This can cause issues as there’s a loading order. I.e.

import PyAp as Ap
import PyDb as Db 

and you will get

Command: PYLOAD PyErr extension class wrapper for base class class PyDbTransactionManager has not been created yet ,Traceback - At (M:\Dev\Projects\PyRxGit\PySamples_doit.py:2): pyload failed:

pyrx_onload. IMHO, that’s too many conditions, too many ways bugs can occur. People will have pyrx_onload files all over and one will eventually contain a stale condition. It should be in one place next to the main module, pyrx_onload can evaluate if it’s being launched from a venv or some other condition.

[gswifort]

Ok, you're quite right about pyrx_onload. So I would stick to the scenarios:

• venv: pyrx_onload.py located in the working directory (os.getcwd()), I am not in favor of placing the file next to the local RxLoader*.*rx because it would require interference with the installed package; assuming I create a project whose dependency is pyrx it will be installed (e.g. with PyPI) "as is", the user would have to manually modify the file in site-packages which is a very bad practice,

• global: pyrx_onload.py loaded from %localappdata%\Programs\PyRx\Bin\ - I don't have a specific opinion here, but I would probably be inclined not to load this file if we run CAD from venv and we won't find the file there; However, if we assume that this file is loaded second, we need to add a function/constant that will inform the user whether he is in a virtual environment, e.g.:

  # pyrx/__init__.py
  import os
  
  def is_venv():
      return os.getenv("VIRTUAL_ENV", None) is not None
  
  IS_VENV = is_venv()

  # pyrx_onload.py
  from pyrx import IS_VENV
  
  def OnPyInitApp() -> None:
      if not IS_VENV:
          ...  # load global modules
      else:
          ...  # load local modules
      #  then complete common tasks

• I would also consider specifying a specific path in the environment variable (if the path does not exist, we do not search further, we print an error/warning)

isort - yes, it sorts imports, where sorting should be skipped, we use the comment # isort: skip, I will check if it can be disabled for specific files.

black - it's a code formatter (not a linter) - it takes care of what PEP8 requires (although the line length mentioned is set to 88 characters by default), you don't have to remember it (although it's worth familiarizing yourself with). I know, the PEP8 requirements seem excessive, but if we don't have to take care of them ourselves, they are very good. I will also check if it can be disabled for specific files.

[CEXT-Dan]

An env variable to disable pyrx_onload would be good to have, something like PYRX_DISABLE_ONLOAD = True. If someone wants to debug a system without extra stuff loaded, it would be a good idea to have a way to turn it off.

[gswifort]

An env variable to disable pyrx_onload would be good to have, something like PYRX_DISABLE_ONLOAD = True. If someone wants to debug a system without extra stuff loaded, it would be a good idea to have a way to turn it off.

I agree - I had a similar idea, but I confirm that it is better to do it with a separate variable and not the same one as for setting the path of the pyrx_onload file.

[CEXT-Dan]

Yes, your idea, just inverted I added PYRX_DISABLE_ONLOAD, looks for “1” as a string, instead of “True”

 std::wstring buffer(5, 0);
 GetEnvironmentVariable(_T("PYRX_DISABLE_ONLOAD"), buffer.data(), buffer.size());;
 if (std::stoi(buffer) == 1)
     return;

[gswifort]

Skipping file formatting:

.vscode\settings.json:

{
    "isort.args": [
        "--skip-glob",
        "PyRxStubs/*.py*"
    ],
    "black-formatter.args": [
        "--force-exclude",
        "^/PyRxStubs/.*pyi?"
    ]
}

isort works with glob and black with re expressions

[gswifort]

So we can implement this when you make changes to pyrx_onload?

[gswifort]

You can use twine to publish to PyPI.

[CEXT-Dan]

Thinking about the formatting, all the ActiveX files, I.e. AxApp25.py, BxApp24.py, should be ignored, not reformatted. These are the ones that are generated with win32com’s makepy. For each new release I use WinMerge to move in the changes from the previous release, it’s a tedious task that takes hours. .PYI files are auto generated too, but I don’t have any issues with these being formatted

As a first step, there should only be one pyrx_onload.py, for all environments, it should reside next to the main module, I.e. PyRx25.0.arx. I have a couple of reasons for this

1, it’s not 100% ready and I am able to create some weird side effects with regards to timing OnPyLoadDwg, OnPyInitApp and PyRxLisp_mylisp functions load incorrectly on ZwCAD and BricsCAD.

2, I want users, or the installer to be able to overwrite the file, if necessary, without having to hunt down multiple copies.

3, I want to better understand the behavior. I.e. how relative paths and acedfind file will work.

[gswifort]

I don't quite understand what you use winMerge for?

[CEXT-Dan]

The ActiveX API is stable except for the GUIDs, and LCIDs I have hand modified the files to eliminate the need for users to create safe arrays by injecting converters into the function bodies So, when there’s an update from say AxApp24.py to AxApp25.py. I first auto generate the outline with the correct GUIDs, and LCIDs Then winMerge in my changes, carefully lol. In short if they are reformatted it would be a disaster, I would need to revert changes to these files

[gswifort]

pyrx_onload - we still don't have a solution to the problem we discussed here (I don't know if you intended to solve it). If we have modules installed globally (%localappdata%\Programs\...), then main module (e.g. PyRx25.0.arx) is ALWAYS loaded from there, regardless of where I load RxLoader25.0.arx from and whether I have a virtual environment activated or not. As a result, pyrx_onload.py is also loaded from the global directory. I think that the Loader location should not matter because in CAD we can have it automatically loaded when the application starts, but based on the active environment, the main module (PyRx25.0.arx) should be loaded locally or only if not found, globally. Assuming the current packaging structure (if not changed according to this thread), if we have a virtual environment activated, we should look for the global module (PyRx25.0.arx) here: %VIRTUAL_ENV%\Lib\site-packages\pyrx\PyRxBin \

[CEXT-Dan]

is ALWAYS loaded from there, regardless of where I load

This is correct, but only for the first releases. The env PYRX_DISABLE_ONLOAD was added to opt out