gtalarico / revitpythonwrapper

Python Wrapper for the Revit API
134 stars 32 forks source link

rpw.ui.Selection()[0] return an error #32

Closed CyrilWaechter closed 6 years ago

CyrilWaechter commented 6 years ago

Hi, According to Selection docstrings. It is supposed to work :

    >>> from rpw import ui
    >>> selection = ui.Selection()
    >>> selection[0]
    FirstElement

But when I try, I got following error :

>>> import rpw
>>> sel = rpw.ui.Selection()
>>> sel
<rpw:Selection [count:1]>
>>> sel[0]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "E:\FichiersLocauxRevit\revitpythonwrapper\revitpythonwrapper.lib\rpw\db\collection.py", line 176, in __getitem__
KeyError: 0
htlcnn commented 6 years ago

Please make sure you have the latest version of rpw. I couldn't reproduce your Exception. And make sure you type these lines of code in RevitPythonShell running inside Revit.

CyrilWaechter commented 6 years ago

Hi htlcnn, Thanks for testing this. I was on dev/main branch which could explain why you were no able to reproduce the error. I did an hard reset on master branch and it solved the issue. It could be related to the deprecated warning message :

[WARNING] ElementSet.as_element_id_list has been depracated and will be removed soon. Use
ElementSet.get_element_ids(as_list=True) instead
<rpw:FamilyInstance [symbol:3D] [id:2051280]>

Probably an unfinished update. @gtalarico just close the issue if it is the case.

gtalarico commented 6 years ago

Sorry for the confusion! Last bit of work I was trying to do some clean up and move set the stage for some of the changes I want to make on 2.0

As part of this clean up, I had one small breaking change, which was I changed the elementSet collection to be an actual set. Because sets are unordered, getting by index is not possible and might not be consistent. Take a look at the element set class, open to suggestions.

Selection behaves like a set in the sense that no duplicates are allowed, so it inherits from set. I am not sure if selection has an order or not, but if it does, I might need to rework this to handle order

CyrilWaechter commented 6 years ago

Without rpw to retrieve selection I was using the following (from Revit 2016 until now) as defined in RPS init.py file :

[doc.GetElement(id) for id in __revit__.ActiveUIDocument.Selection.GetElementIds()]

Where :

__revit__.ActiveUIDocument.Selection.GetElementIds() # return a List[ElementId]() which is an ordered list. You can get First from or access it by index.
doc.GetElement(id) # return a single element

It looks like there is no ordered set in python (only ordered dict). In what I understand of what you are trying to do. Maybe you could pass through a set only when you are trying to add/set a selection ?

gtalarico commented 6 years ago

@CyrilWaechter Do you know if the order returned by revit.ActiveUIDocument.Selection.GetElementIds() is consistent or related to how a user selects things? (order?) I think if the answer to both questions is now, Selection should probably remain a set and getting by index would not make sense.

If selection is consistent or ordered, then maybe I should undo the work and got back to what I had before (ElementSet was previsouly using an OrderedDict to store elementId:element pairs. This ensured uniqueness but was not a true set.

CyrilWaechter commented 6 years ago

@gtalarico __revit__.ActiveUIDocument.Selection.GetElementIds() return a List[ElementId]() ordered by Id no matter in what order you select elements.

Just thinking out loud but maybe they return a list of element ids instead of elements themselves to keep selection light. Storing large amount of elements (some like families which can be >1 MB) is maybe not so much a good idea. So yes OrderedDict is probably a good solution as far as we do not manipulate too much objects. And later I think we can find a way in python to store elements Ids only but return elements instead of Id when you iterate through it (loop or index).

gtalarico commented 6 years ago

image

Fixed