BlockResearchGroup / compas_tna

Form finding of funicular networks in compression with vertical loads using Thrust Network Analysis
https://blockresearchgroup.github.io/compas_tna
MIT License
13 stars 9 forks source link

Check for top-level imports when running on ironpython #12

Closed arpastrana closed 4 years ago

arpastrana commented 5 years ago

Describe the nature of the PR

Attempt to import public modules from compas_tna.equilibrium or compas_tna.utilities.diagrams raises an exception on ironpython (rhino / gh) as it cannot find compas.numerical.

This issue is related to the import scheme of compas 0.7.2, which deliberately omits a few modules (such as matrices.py) in compas.numerical if it detects that compas.IPY = True in its __init__.py.

Thus, I have re-arranged the top-level imports to "load" only the ironpython supported libraries (which nonetheless extends to a few other numpy/scipy methods that where handled through a try / except conditional).

I have tested the proposed changes with a couple of scripts from HiLo_floors, and they worked nicely as before (when I was working with compas 0.6.2).

To Reproduce

# in ironpython
from compas_tna.diagrams import FormDiagram  # works
from compas_tna.equilibrium import horizontal_proxy  # does not work, raises exception

Desktop:

tomvanmele commented 4 years ago

hi raphael,

sounds good, but these things need to be handled in the __init__ files. this way there is only location where these ironpython checks need to happen.

thanks, tom

arpastrana commented 4 years ago

Hi Tom, thanks for the feedback. Well, in this case, what if we put the _proxy methods in separate files within the module, so that only those get imported if compas.IPY? This is already done in compas.numerical, for example.

Files in the folder would look like:

horizontal.py
vertical.py
horizontal_proxy.py
vertical_proxy.py

On the other hand, since all the equilibrium _proxy methods have from_data -> to_data operation in common; would it make sense to encapsulate this functionality in a method part of e.g. compas_tna.utilities?

I implemented a selector and diagram-adapter logic along these lines compas_floors.analysis.tna (https://github.com/BlockResearchGroup/HiLo_floors/blob/master/src/compas_floors/analysis/tna.py - lines 167-172); but in general it generalizes the process as follows:

def _equilibrate(diagrams, callback, **kwargs):
    '''
    '''
       # converts method into a + _proxy one if compas.IPY
    _eq = _equilibrium_func_selector()
       # performs Diagram().to_data() if compas.IPY
    diagrams = adapt_diagrams(diagrams, callback)
      # executes
    return _eq(diagrams, callback, **kwargs) 

(I should probably refactor and just check for compas.IPY only once).

arpastrana commented 4 years ago

Now we make the compas.IPY checks in the __init__ files.

Since the compas_tna.equilibrium methods are meant to be run only outside of ironpython, I think it makes sense to make them available only if compas.IPY is False. Thus, the numpy and scipy imports at the top remain there without further checks.

So it goes with compas_tna.utilities.diagrams