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

Allow overwrite of constructor arguments to return None #178

Open
5 tasks done
oruebel opened this issue Oct 17, 2019 · 2 comments · May be fixed by #1167
Open
5 tasks done

Allow overwrite of constructor arguments to return None #178

oruebel opened this issue Oct 17, 2019 · 2 comments · May be fixed by #1167
Assignees
Labels
category: bug errors in the code or code behavior category: enhancement improvements of code or code behavior priority: low alternative solution already working and/or relevant to only specific user(s)
Milestone

Comments

@oruebel
Copy link
Contributor

oruebel commented Oct 17, 2019

When customizing in the ObjectMapper the mapping for a constructor argument, the mapping is ignored if the function returns None. E.g.,

@register_map(Sweeps)
class SweepsMap(DynamicTableMap):
    @ObjectMapper.constructor_arg('intracellular_recordings')
    def intracellular_recordings_arg(self, builder, manager):
          return None

will always be ignored. This is due to the following:

hdmf/src/hdmf/build/map.py

Lines 1204 to 1210 in 44c02ea

override = self.__get_override_carg(argname, builder, manager)
if override is not None:
val = override
elif argname in const_args:
val = const_args[argname]
else:
continue

Here the check if override is not None: is used to check if a constructor argument is overwritten or not. Instead the check should look for whether a custom mapping is defined, rather than using the None return value as an indicator that there is no overwrite.

Proposed solution Add function has_overwrite_carg to check if a constuctor argument is overwritten and use it instead.

Checklist

  • Have you ensured the feature or change was not already reported ?
  • Have you included a brief and descriptive title?
  • Have you included a clear description of the problem you are trying to solve?
  • Have you included a minimal code snippet that reproduces the issue you are encountering?
  • Have you checked our Contributing document?
@oruebel oruebel added category: bug errors in the code or code behavior category: enhancement improvements of code or code behavior labels Oct 17, 2019
@oruebel oruebel self-assigned this Oct 17, 2019
@rly rly added this to the Next Release milestone Jan 5, 2023
@rly rly assigned rly and unassigned oruebel Jan 5, 2023
@rly
Copy link
Contributor

rly commented Jan 5, 2023

This is a quick fix. Will do and add test for next release.

@rly rly added the priority: low alternative solution already working and/or relevant to only specific user(s) label Jan 5, 2023
@mavaylon1
Copy link
Contributor

@rly is this something that we want before the major release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: bug errors in the code or code behavior category: enhancement improvements of code or code behavior priority: low alternative solution already working and/or relevant to only specific user(s)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants