dave-howard / vsdx

vsdx - A python library for processing .vsdx files
BSD 3-Clause "New" or "Revised" License
70 stars 25 forks source link

Master-to-Shape inheritance #4

Closed james-powis closed 3 years ago

james-powis commented 3 years ago

For deduplication shapes can be inherited and then overridden at the page level.

Take the following example file: test4_lines.zip

2 of the shapes are declared (text) in master1 and master2 while 2 are in page1.

https://docs.microsoft.com/en-us/openspecs/sharepoint_protocols/ms-vsdx/74428617-9833-4d73-aa7f-f3a6f043a12d

james-powis commented 3 years ago

Dave,

Carrying the discussion from #3 here to remain on topic. I have created a fork which rounds out some of the basic python module structure and best practices (ie: requirements file, unit_tests in unit_tests for pytest discovery, flake8 config etc)

Additionally I wrote a (failing) unit test for module inheritance as well as initial prerequisites for master inheritance.

The code can be found here: https://github.com/james-powis/vsdx/tree/add_connection_functionality

Unfortunately my brain does not work in the way that ETREE requires it to work to some how figure out the secondary lookup strategy / merging between shape and referenced master all the while preserving the ability to save the VSDX file without breaking apart the efficiencies provided in the file format specifications.

I do not believe that this fork is up to merge request quality as it is not a finished solution, and I must move on to other projects now that I have answered the POC question of "can it be done, yes but not easily", however I wanted to share my efforts with you should they be of use.

The test4_lines.vsdx was generated using lucid chart, and contains SHAPE_A label defined in the page1.xml where as the SHAPE_B shape in page1.xml references the shape defined in master1.xml. The test_case provided SHOULD pass if inheritance is working properly... I just can't figure out a great way to handle the merge / lookup strategy which is functional without zeroing the master on file write (which might be an option, but could in some situations massively inflate the resulting file size).

Please let me know if I can be of any assistance in furthering the resolution of this issue (and in the discussion on #3), I know my boss got more excited then he should have when I described the potential for using a visio file as the entry point towards some of our automation endeavors.

james-powis commented 3 years ago

Also I did make some changes to use os.path functions to start the support for Windows based systems some time in the near future, as well as resolving absolute pathing from relative.

dave-howard commented 3 years ago

James - that's great, I wasn't clear on the master inheritance issue so a failing test is perfect thanks. I will take a look at both test and other changes in your fork. Thanks Dave

dave-howard commented 3 years ago

Hi James - the latest update now loads master pages and master ID for each page. I've not actually been able to map the Shape to master shape so test still fails. However - just thought I would note the progress made here. Need to investigate the vsdx file format more to understand that relationship.

stevensultana commented 3 years ago

Hello, I've messed around a bit with this - I think managing to get Master-to-Shape inheritance to work will be quite a break for the package.

I have a demo for getting cells/data from masters. Setting Cells is a bit of a beast - you can see I tried to start something, but quickly tracked back: https://github.com/stevensultana/vsdx/tree/quick_inheritance

My approach is the following: A Shape's Master attribute refers to the ID of a Master element in the masters.xml. The Master element also has an rId, which is used to get the master page where the master shape resides (masterN.xml). So:

From the Shape, when trying to get a cell value, if the cell is not defined:

Similarly for getting the Text.

For what it's worth, James's test now passes. But I think it's best to craft a couple of other tests as well. Not to mention implementing sub_shapes, and implementing setting of data. Putting this here in case there is already some work going on for this.

dave-howard commented 3 years ago

I think after the changes @stevensultana has added and the tests @dave-howard has added in last commit means we can close this issue - will leave open for a week for comments and otherwise close after.

dave-howard commented 3 years ago

Closing this one as done