Skip to content

Update tag decorator to support attributes with string value#129

Merged
jamesthejellyfish merged 1 commit intoHuawei-CPLLab:mainfrom
Narutoworld:update-decorator-tag
Mar 4, 2026
Merged

Update tag decorator to support attributes with string value#129
jamesthejellyfish merged 1 commit intoHuawei-CPLLab:mainfrom
Narutoworld:update-decorator-tag

Conversation

@Narutoworld
Copy link
Contributor

This patch update tag decorator to support attributes that has string value.

@Narutoworld Narutoworld force-pushed the update-decorator-tag branch from 7f1a301 to c7056c3 Compare March 4, 2026 14:08
Copy link
Collaborator

@jamesthejellyfish jamesthejellyfish left a comment

Choose a reason for hiding this comment

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

Mostly LGTM. Are you able to add a simple testcase for this behaviour? Just to make sure that nothing we do down the line messes this up, and as an example of how to use this. Also we could probably combine this functionality with int_attr so that we just use tag for everything (maybe a keyword argument for an optional type? I know int_attr right now assumes Index but that isn't always ideal), but that could be another PR later down the line, so it's not necessary to do right now

@Narutoworld Narutoworld force-pushed the update-decorator-tag branch from c7056c3 to 4626414 Compare March 4, 2026 16:13
@classmethod
def CType(cls) -> tuple[type]:
ctypes_map = {
16: np.float16,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this might be a change from a different PR creeping in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, good catch

@Narutoworld Narutoworld force-pushed the update-decorator-tag branch from 4626414 to 3784951 Compare March 4, 2026 16:42
@@ -1,5 +1,6 @@
from __future__ import annotations

import numpy as np
Copy link
Collaborator

Choose a reason for hiding this comment

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

this numpy import is still here from earlier

@Narutoworld Narutoworld force-pushed the update-decorator-tag branch from 3784951 to c63e1a5 Compare March 4, 2026 17:47
Copy link
Collaborator

@jamesthejellyfish jamesthejellyfish left a comment

Choose a reason for hiding this comment

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

LGTM

@jamesthejellyfish jamesthejellyfish merged commit e4e01ad into Huawei-CPLLab:main Mar 4, 2026
2 checks passed
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 this pull request may close these issues.

2 participants