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

[WIP] Remove the use of meta classes in the ProcessBlock class hierarchy #1404

Closed
wants to merge 4 commits into from

Conversation

jsiirola
Copy link
Contributor

@jsiirola jsiirola commented May 3, 2024

Fixes

Summary/Motivation:

The use of a metaclass for defining process blocks was both confusing and caused unintentional errors (conflicting metaclass definitions that faught with the RenamedClass backwards compatibility wrappers. This PR reworks the magic for declaring process block classes to:

  • remove the use of metaclasses
  • declare unique instances of the Scalar and Indexed block classes
    • ensure the Scalar and Indexed classes are added to the module scope (along with the generic derived Block object)
  • improve general documentation and code clarity

Changes proposed in this PR:

  • (see above)

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

@jsiirola jsiirola changed the title Remove the use of meta classes in the ProcessBlock class hierarchy [WIP] Remove the use of meta classes in the ProcessBlock class hierarchy May 5, 2024
# the arguments, we need to map this to either the
# ScalarProcessBlock aor IndexedProcessBlock subclass.
if not args or (args[0] is UnindexedComponent_set and len(args) == 1):
return super().__new__(cls._scalar_process_block, *args, **kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return super().__new__(cls._scalar_process_block, *args, **kwargs)
# pylint: disable-next=no-member
return super().__new__(cls._scalar_process_block, *args, **kwargs)

if not args or (args[0] is UnindexedComponent_set and len(args) == 1):
return super().__new__(cls._scalar_process_block, *args, **kwargs)
else:
return super().__new__(cls._indexed_process_block, *args, **kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return super().__new__(cls._indexed_process_block, *args, **kwargs)
# pylint: disable-next=no-member
return super().__new__(cls._indexed_process_block, *args, **kwargs)

@andrewlee94
Copy link
Member

Closing this as low priority for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:Low Low Priority Issue or PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants