Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
mpsonntag opened this issue Aug 11, 2021 · 4 comments · Fixed by #532
Closed

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

mpsonntag opened this issue Aug 11, 2021 · 4 comments · Fixed by #532

Comments

@mpsonntag
Copy link
Contributor

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:

newentity = super(Block, cls).create_new(nixfile, nixparent, h5parent,

if name and type_:

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

@mpsonntag mpsonntag changed the title Creating a block with "name" only ignores the "name" value Creating a block with only "name" provided ignores the "name" value Aug 31, 2021
@jgrewe
Copy link
Member

jgrewe commented Oct 26, 2021

the behaviour is odd in more than one way:

  • if name and type are not none, they will be checked and it either of them is an empty string there will be an exception (which is the desired behaviour)
  • if either of them is none, name will equal the id, and type_ will not be checked.

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
Copy link
Member

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
Copy link
Member

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

@jgrewe
Copy link
Member

jgrewe commented Oct 26, 2021

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants