Python Packaging #86
Replies: 15 comments 6 replies
-
I have taken preliminary steps to pack and distribute the package.
All you need to do to build the package is activate the virtual environment and run the A few things to discuss:
|
Beta Was this translation helpful? Give feedback.
-
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. The only real requirements I have are. Black is a linter, right? I had installed super-linter and it kicked out all the autogenerated ActiveX stuff. Regarding isort, this changes the order of the imports, correct? This can cause issues as there’s a loading order.
and you will get
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. |
Beta Was this translation helpful? Give feedback.
-
Ok, you're quite right about pyrx_onload. So I would stick to the scenarios:
isort - yes, it sorts imports, where sorting should be skipped, we use the comment 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. |
Beta Was this translation helpful? Give feedback.
-
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. |
Beta Was this translation helpful? Give feedback.
-
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 |
Beta Was this translation helpful? Give feedback.
-
Yes, your idea, just inverted std::wstring buffer(5, 0);
GetEnvironmentVariable(_T("PYRX_DISABLE_ONLOAD"), buffer.data(), buffer.size());;
if (std::stoi(buffer) == 1)
return; |
Beta Was this translation helpful? Give feedback.
-
Skipping file formatting: .vscode\settings.json: {
"isort.args": [
"--skip-glob",
"PyRxStubs/*.py*"
],
"black-formatter.args": [
"--force-exclude",
"^/PyRxStubs/.*pyi?"
]
}
|
Beta Was this translation helpful? Give feedback.
-
So we can implement this when you make changes to |
Beta Was this translation helpful? Give feedback.
-
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. |
Beta Was this translation helpful? Give feedback.
-
I don't quite understand what you use winMerge for? |
Beta Was this translation helpful? Give feedback.
-
The ActiveX API is stable except for the GUIDs, and LCIDs |
Beta Was this translation helpful? Give feedback.
-
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). |
Beta Was this translation helpful? Give feedback.
-
This is correct, but only for the first releases. The env PYRX_DISABLE_ONLOAD was added to opt out |
Beta Was this translation helpful? Give feedback.
-
The name |
Beta Was this translation helpful? Give feedback.
-
Originally posted by @gswifort in #67 (reply in thread)
Beta Was this translation helpful? Give feedback.
All reactions