search

MPh-py / MPh

Pythonic scripting interface for Comsol Multiphysics
https://mph.readthedocs.io
MIT License
263 stars 67 forks source link

`model.remove()` can fail silently #33

Closed max3-2 closed 3 years ago

max3-2 commented 3 years ago

Hi,

the method model.remove() seems to fail silently if the tag is not available. While this does not break anything, it can lead to strange behavior.

Additionally, this only works in root groups defined in self_groups since sub groups won't always have .remove()

This could be fixed with a check if the tag is in the list of tags. Otherwise, I will post an example for hierarchy soon which, among others features solves this issue.

john-hen commented 3 years ago

I don't quite know what you mean ("tag is not available"), but since this part of the code base is currently in flux, I think it will sort itself out eventually.

max3-2 commented 3 years ago

... and now I cant follow you regarding

"tag is not available"

with my quick testing, you could call (group dosn‘t matter, old API)model.remove(‘functions‘, ‘interpolation‘) no matter if the node interpolation exists. If not available, it just does nothing. While this is not strictly an error, you would have a hard time tracing this on e.g. typos

john-hen commented 3 years ago

I agree that it shouldn't fail silently. I just don't see how it does. Maybe I'm missing something. With current master (as model.remove() is new and not in the current release), I get this:

>>> import mph
>>> client = mph.start()
>>> model = client.load('tests/capacitor.mph')
>>> model.remove('functions', 'test_function')
>>> model.remove('functions', 'test_function')
No node named "test_function" in group "functions".
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "d:\home\coding\MPh\mph\model.py", line 411, in remove
    tag = self._node(group, node).tag()
  File "d:\home\coding\MPh\mph\model.py", line 116, in _node
    raise LookupError(error) from None
LookupError: No node named "test_function" in group "functions".

So it works the first time, because "test_function" exists in the demo model, and fails the second time, after that function has been removed. It also fails for any other name I pass for the second argument.

max3-2 commented 3 years ago

I double checked and can not reproduce. My bad. The reason I got confused is that this can happen when calling the java.remove() which some elements have - this does not check anything nor raises an error, e.g. model._group('physics')().remove('notthere') - however this is nothing we can catch.