Port to wx python 4.2.1 and Python3 #2

chrisinabox · 2023-11-25T05:26:07Z

Updates the OD Edit tool to use the latest wxPython version. The UI now works with Python 3.
This change does not do any significant refactoring to improve the underlying logic.

Able to open UI but cannot open files yet

Update error window message so it's more generic

Add new parser so no need to use eval. Add tests for basic interpreter

…backs

Change default value for number field from 0 to 1 Add check for number >0 before allowing return OK

…with-zero-elements-creates-ghost-data-in-od Fix ghost data created in OD files if number left as 0 in MapVariableDialog

Correct mistakes in comments

…-from-object-type-record Fix unable to remove sub-indexes from record types

…on 2 Update README to remove python 2 references

Refactor FileDialog for SaveAs to use wxPython recommended implementation Update wildcards for easier selection of OD type

…type-od-without-closing-the-file-and-opening-again Fix filepath not set on SaveAs success

sveinse

This PR is a long awaited contribution. Thank you very much @chrisinabox . I really appreciate the efforts and it will be a great addition to the tool. However, we're not quite there yet and some changes are needed.

AFAICS there are three changes included in this PR:

wx support for py3
The removal of py2 (at least in the comments and setup)
Improvements on string parsing

These three belongs in separate PRs. Please adjust the PR accordingly. I recommend making this the wx change and make the two other changes later. If the string parsing is a prerequisite for the wx change, I recommend filing a new PR for that and that it will be approved before this.

The python package code check failed. It does not seem to be able to install wx without compilation. Please check the logs. Getting this working is also something that must be fixed.

README.md

src/objdictgen/node.py

src/objdictgen/ui/subindextable.py

tests/test_node.py

chrisinabox · 2023-11-27T00:32:03Z

This PR is a long awaited contribution. Thank you very much @chrisinabox . I really appreciate the efforts and it will be a great addition to the tool. However, we're not quite there yet and some changes are needed.

AFAICS there are three changes included in this PR:

wx support for py3

The removal of py2 (at least in the comments and setup)

Improvements on string parsing

These three belongs in separate PRs. Please adjust the PR accordingly. I recommend making this the wx change and make the two other changes later. If the string parsing is a prerequisite for the wx change, I recommend filing a new PR for that and that it will be approved before this.

The python package code check failed. It does not seem to be able to install wx without compilation. Please check the logs. Getting this working is also something that must be fixed.

I believe your summary is correct on the most part. In addition though I have also implemented a couple of small bug fixes to the UI which are now part of this pull request. These changes are:

Allowing deletion of record types objects from the manufacturer specific information section for a node
Remove a bug that added "ghost" data to the UserMapping dictionary when adding manufacturer info to the node

…for python 2" This reverts commit 3d1f1c2.

This paritally reverts commit 377f3bc. Removes refactoring of node.py to remove use of ast methods, reintroduces eval().

chrisinabox · 2023-11-27T02:26:29Z

@sveinse The workflows have been updated to include the required dependencies for building wxPython. While this does now build successfully, unfortunately the build times are relatively significant. This is primarily an issue for Linux systems, documented here - https://wxpython.org/pages/downloads/index.html

sveinse · 2023-11-27T09:55:06Z

The check passed with you change 👍. However it took 23 minutes (!) where compiling wx took 22 of them. I think we need to pursue a better ways to do this. The problem is not that it takes CPU time, but its building something that isn't ours. Need to look into if there is a way to cache wx.

sveinse

I haven't run the code yet, but it looks good

src/objdictgen/nodemanager.py

sveinse · 2023-11-27T15:02:46Z

I've run the PR on windows and briefly tested it. It works out of box. 🥇 great job. So I'm giving a green light to the PR ✔️ .

Please make the two following changes before merging:

Figure out what you want to do on the two except statement comment above
Bump odg to version 3.4 (search for 3.3 and replace with 3.4)

chrisinabox · 2023-11-27T15:10:52Z

I've run the PR on windows and briefly tested it. It works out of box. 🥇 great job. So I'm giving a green light to the PR ✔️ .

Please make the two following changes before merging:

Figure out what you want to do on the two except statement comment above

Bump odg to version 3.4 (search for 3.3 and replace with 3.4)

Amazing! Shall get these done ASAP.

I have only tested on a windows system also.

Adjust try/catch in nodemanager GetNodeEntryValues(...) for slightly better (but not good) error handling

chrisinabox · 2023-11-27T16:30:50Z

I have tested this in a Ubuntu VM and it all appears to be working as intended. There are a number of warnings generated by gtk but this does not appear to effect the functionality.

chrisinabox and others added 15 commits November 24, 2023 01:49

Refactor majority of UI to use wxPython 4.2.1

6a71fce

Able to open UI but cannot open files yet

Fix depreciated methods AddWindow and AddSizer. Replaced with Add(..)

8e50da4

Fix various wx function/object changes

f57603f

Update error window message so it's more generic

Fix unable to change table values due to wrong callback used

44bcbce

Fix UI bugs which prevent rendering

377f3bc

Add new parser so no need to use eval. Add tests for basic interpreter

Fix FileDialog styling

c6b7f75

Fix number of elements in MainSizer for NodeInfo edit

fd68d1b

Fix bug introduced in porting where wrong IDs used for OK button call…

d4267e2

…backs

Updates to MapVariableDialog

c8880b6

Change default value for number field from 0 to 1 Add check for number >0 before allowing return OK

Merge pull request #4 from chrisinabox/1-new-record-or-array-objects-…

b83eb8b

…with-zero-elements-creates-ghost-data-in-od Fix ghost data created in OD files if number left as 0 in MapVariableDialog

Fix bug in conditional in RemoveSubentriesFromCurrent

1a9f217

Correct mistakes in comments

Merge pull request #5 from chrisinabox/3-ui-cannot-delete-sub-indexes…

06b2990

…-from-object-type-record Fix unable to remove sub-indexes from record types

Update setup to include wxPython packages and remove support for pyth…

3d1f1c2

…on 2 Update README to remove python 2 references

Fix filepath not set on SaveAs success

e0bb800

Refactor FileDialog for SaveAs to use wxPython recommended implementation Update wildcards for easier selection of OD type

Merge pull request #7 from chrisinabox/2-ui-cant-save-new-nodes-with-…

6757d01

…type-od-without-closing-the-file-and-opening-again Fix filepath not set on SaveAs success

sveinse requested changes Nov 26, 2023

View reviewed changes

chrisinabox added 4 commits November 26, 2023 16:56

Revert "Update setup to include wxPython packages and remove support …

8afaef5

…for python 2" This reverts commit 3d1f1c2.

Partial revert "Fix UI bugs which prevent rendering"

9168780

This paritally reverts commit 377f3bc. Removes refactoring of node.py to remove use of ast methods, reintroduces eval().

Add wxPython to setup dependancies

08fa269

Update workflow yml to include required build dependancies

982c830

sveinse reviewed Nov 27, 2023

View reviewed changes

src/objdictgen/nodemanager.py Show resolved Hide resolved

Bump version to 3.4

4693e72

Adjust try/catch in nodemanager GetNodeEntryValues(...) for slightly better (but not good) error handling

chrisinabox requested a review from sveinse November 27, 2023 16:25

sveinse approved these changes Nov 27, 2023

View reviewed changes

sveinse merged commit dee2860 into Laerdal:main Nov 27, 2023

chrisinabox deleted the feature-port-to-wxPython-4_2_1-and-python3 branch November 28, 2023 00:19

sveinse mentioned this pull request Nov 28, 2023

Remove python 2 references #3

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port to wx python 4.2.1 and Python3 #2

Port to wx python 4.2.1 and Python3 #2

chrisinabox commented Nov 25, 2023

sveinse left a comment •

edited

Loading

chrisinabox commented Nov 27, 2023

chrisinabox commented Nov 27, 2023 •

edited

Loading

sveinse commented Nov 27, 2023

sveinse left a comment

sveinse commented Nov 27, 2023

chrisinabox commented Nov 27, 2023 •

edited

Loading

chrisinabox commented Nov 27, 2023

Port to wx python 4.2.1 and Python3 #2

Port to wx python 4.2.1 and Python3 #2

Conversation

chrisinabox commented Nov 25, 2023

sveinse left a comment • edited Loading

Choose a reason for hiding this comment

chrisinabox commented Nov 27, 2023

chrisinabox commented Nov 27, 2023 • edited Loading

sveinse commented Nov 27, 2023

sveinse left a comment

Choose a reason for hiding this comment

sveinse commented Nov 27, 2023

chrisinabox commented Nov 27, 2023 • edited Loading

chrisinabox commented Nov 27, 2023

sveinse left a comment •

edited

Loading

chrisinabox commented Nov 27, 2023 •

edited

Loading

chrisinabox commented Nov 27, 2023 •

edited

Loading