Skip to content

Fix TypeError in by_name() when name is not found #83

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.

Sign up for GitHub

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

Jump to bottom
Merged
apdavison merged 3 commits into from
Jan 9, 2026
Merged

Fix TypeError in by_name() when name is not found #83

apdavison merged 3 commits into from
Jan 9, 2026

Conversation

@ree-gupta
Copy link

@ree-gupta ree-gupta commented Jan 8, 2026 •
edited
Loading

Fixes a TypeError in the by_name() method when searching for a name that doesn't exist in the instance lookup cache.

The bug: When using match="equals" (the default), by_name() returns None from dict.get(name, None) if the name isn't found. This causes len(matches) to raise:

TypeError: object of type 'NoneType' has no len()

The fix: Change the default value from None to [] (empty list), making it consistent with the match="contains" branch which already initializes matches = [].

How I found this

While using bids2openminds with openminds 0.4.0, the tool crashed when looking up technique names that weren't in the cache.

The Traceback:

Traceback (most recent call last):
  File "/home/gupta/Work/EBRAINS/bids2openminds/.venv/bin/bids2openminds", line 10, in <module>
    sys.exit(convert_click())
             ~~~~~~~~~~~~~^^
  File "/home/gupta/Work/EBRAINS/bids2openminds/.venv/lib/python3.13/site-packages/click/core.py", line 1485, in __call__
    return self.main(*args, **kwargs)
           ~~~~~~~~~^^^^^^^^^^^^^^^^^
  File "/home/gupta/Work/EBRAINS/bids2openminds/.venv/lib/python3.13/site-packages/click/core.py", line 1406, in main
    rv = self.invoke(ctx)
  File "/home/gupta/Work/EBRAINS/bids2openminds/.venv/lib/python3.13/site-packages/click/core.py", line 1269, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/gupta/Work/EBRAINS/bids2openminds/.venv/lib/python3.13/site-packages/click/core.py", line 824, in invoke
    return callback(*args, **kwargs)
  File "/home/gupta/Work/EBRAINS/bids2openminds/bids2openminds/converter.py", line 81, in convert_click
    convert(input_path, save_output=True, output_path=output_path,
    ~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
            multiple_files=multiple_files, include_empty_properties=include_empty_properties, quiet=quiet)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/gupta/Work/EBRAINS/bids2openminds/bids2openminds/converter.py", line 44, in convert
    dataset_version = main.create_dataset_version(
        bids_layout, dataset_description, layout_df, subjects_list, file_repository, behavioral_protocols, collection)
  File "/home/gupta/Work/EBRAINS/bids2openminds/bids2openminds/main.py", line 233, in create_dataset_version
    techniques = create_techniques(layout_df)
  File "/home/gupta/Work/EBRAINS/bids2openminds/bids2openminds/main.py", line 132, in create_techniques
    openminds_techniques_cache = techniques_openminds(suffix)
  File "/home/gupta/Work/EBRAINS/bids2openminds/bids2openminds/main.py", line 116, in techniques_openminds
    openminds_obj = openminds_type.by_name(item_openminds)
  File "/home/gupta/Work/EBRAINS/bids2openminds/.venv/lib/python3.13/site-packages/openminds/v3/controlled_terms/technique.py", line 156, in by_name
    elif len(matches) > 0:
         ~~~^^^^^^^^^
TypeError: object of type 'NoneType' has no len()

@ree-gupta
Fix TypeError in by_name() when name is not found
cdb30ef 
When using `match="equals"` (the default), `by_name()` would return
`None` from `dict.get(name, None)` if the name wasn't found in the
lookup cache. This caused `len(matches)` on line 51 to raise:

    TypeError: object of type 'NoneType' has no len()

This fix changes the default value from `None` to `[]` (empty list),
making it consistent with the `match="contains"` branch which already
initializes `matches = []`.
@ree-gupta ree-gupta requested a review from apdavison January 8, 2026 16:20
@apdavison
Copy link
Member

apdavison commented Jan 8, 2026

Thanks for reporting and fixing this @ree-gupta

This fix points out another flaw with this method - if no match is found, it sometimes returns None and sometimes an empty list. We should pick one or the other. For consistency with fairgraph, I suggest always returning None if no matches are found.

Could you update the PR accordingly?

@ree-gupta
Copy link
Author

ree-gupta commented Jan 8, 2026

Could you update the PR accordingly?

Thanks for the feedback @apdavison. This should hopefully fix it.

@apdavison
Copy link
Member

apdavison commented Jan 8, 2026

That's an elegant implementation!

One final thing, please could you add a regression test for this, either extending test_issue0069 or adding a new test in the same file?

@ree-gupta
Copy link
Author

ree-gupta commented Jan 9, 2026 •
edited
Loading

That's an elegant implementation!

One final thing, please could you add a regression test for this, either extending test_issue0069 or adding a new test in the same file?

I added new test. One CI check has been running for 10 minutes 👀 I hope it passes 😅

edit: passes now 🎉

@apdavison apdavison merged commit 964ba4e into openMetadataInitiative:pipeline Jan 9, 2026
3 checks passed
apdavison added a commit that referenced this pull request Jan 9, 2026
@apdavison
update author list following merge of #83
0f9a729
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@apdavison apdavison apdavison approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ree-gupta @apdavison