G-Node / nixpy

Python library for NIX
https://readthedocs.org/projects/nixpy
Other
19 stars 26 forks source link

Creating a block with only "name" provided ignores the "name" value #524

Closed mpsonntag closed 2 years ago

mpsonntag commented 3 years ago

When creating a block and providing only the name, but not the type, the name value is ignored and the block name defaults to the blocks ID.

e.g. b = nf.create_block(name="I-shall-be-ignored")

The corresponding lines responsible can be found in block.py and entity.py:

https://github.com/G-Node/nixpy/blob/c5ffc3b6b6d22b89433acffc9b96a6a0dacfe3e0/nixio/block.py#L55 https://github.com/G-Node/nixpy/blob/c5ffc3b6b6d22b89433acffc9b96a6a0dacfe3e0/nixio/entity.py#L23

Is this expected behavior and what would be the reason for ignoring a name value when the type has not been provided?

jgrewe commented 2 years ago

the behaviour is odd in more than one way:

I would suggest to make sure that type is not empty and if name is not given, it is set to the id.

@achilleas-k is there a need to allow an empty type (in neo-nixio)

achilleas-k commented 2 years ago

This is definitely a bug. The name and type should both be required. The check in the entity was for entities that don't have a name, which at some point was the Feature class. Now though, all Entities have names and the Feature no longer subclasses Entity, so we should actually raise an error when either name or type are None or empty strings.

achilleas-k commented 2 years ago

@jgrewe Need a confirmation for the above because my memory might be broken. Name and Type are required in NIX, right?

jgrewe commented 2 years ago

They should be required. Wasn't sure myself if we decided to allow empty types e.g. in the neo nix io for some reason I forgot... Will fix it. Thanks!