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

Fix some more duplicate enum to_py utility code names #5905

Merged
merged 2 commits into from Feb 10, 2024

Conversation

da-woods
Copy link
Contributor

It's possible to declare an extern enum multiple times, in a way that makes the declaration name the same. Therefore, also prefix declaration names with the module qualified name where possible.

It's possible to declare an extern enum multiple times, in a way
that makes the declaration name the same. Therefore, also prefix
declaration names with the module qualified name where possible.
@da-woods
Copy link
Contributor Author

I'm fairly sure this does fix #5860

@EdwardSro
Copy link

I'm fairly sure this does fix #5860

Yes, it does fix the compilation. Thanks!
But now I see the following errors/warnings when I run our python tests (that uses the cython generated code):

<frozen importlib._bootstrap>:241: UserWarning: enum class rdma_port_space not importable from pyverbs.librdmacm_enums. You are probably using a cpdef enum declared in a .pxd file that does not have a .py or .pyx file. <frozen importlib._bootstrap>:241: UserWarning: enum class ibv_send_flags not importable from pyverbs.libibverbs. You are probably using a cpdef enum declared in a .pxd file that does not have a .py or .pyx file. <frozen importlib._bootstrap>:241: UserWarning: enum class ibv_wr_opcode not importable from pyverbs.libibverbs_enums. You are probably using a cpdef enum declared in a .pxd file that does not have a .py or .pyx file. <frozen importlib._bootstrap>:241: UserWarning: enum class ibv_send_flags not importable from pyverbs.libibverbs_enums. You are probably using a cpdef enum declared in a .pxd file that does not have a .py or .pyx file. <frozen importlib._bootstrap>:241: UserWarning: enum class ibv_ops_wr_opcode not importable from pyverbs.libibverbs_enums. You are probably using a cpdef enum declared in a .pxd file that does not have a .py or .pyx file. <frozen importlib._bootstrap>:241: UserWarning: enum class ibv_ops_flags not importable from pyverbs.libibverbs_enums. You are probably using a cpdef enum declared in a .pxd file that does not have a .py or .pyx file. <frozen importlib._bootstrap>:241: UserWarning: enum class ibv_wq_type not importable from pyverbs.libibverbs_enums. You are probably using a cpdef enum declared in a .pxd file that does not have a .py or .pyx file. <frozen importlib._bootstrap>:241: UserWarning: enum class ibv_values_mask not importable from pyverbs.libibverbs. You are probably using a cpdef enum declared in a .pxd file that does not have a .py or .pyx file. <frozen importlib._bootstrap>:241: UserWarning: enum class ibv_flow_attr_type not importable from pyverbs.libibverbs. You are probably using a cpdef enum declared in a .pxd file that does not have a .py or .pyx file. <frozen importlib._bootstrap>:241: UserWarning: enum class ibv_qp_type not importable from pyverbs.libibverbs_enums. You are probably using a cpdef enum declared in a .pxd file that does not have a .py or .pyx file. <frozen importlib._bootstrap>:241: UserWarning: enum class ibv_qp_state not importable from pyverbs.libibverbs_enums. You are probably using a cpdef enum declared in a .pxd file that does not have a .py or .pyx file. <frozen importlib._bootstrap>:241: UserWarning: enum class ibv_mtu not importable from pyverbs.libibverbs_enums. You are probably using a cpdef enum declared in a .pxd file that does not have a .py or .pyx file. <frozen importlib._bootstrap>:241: UserWarning: enum class mlx5dv_mkey_err_type not importable from pyverbs.providers.mlx5.libmlx5. You are probably using a cpdef enum declared in a .pxd file that does not have a .py or .pyx file. <frozen importlib._bootstrap>:241: UserWarning: enum class mlx5dv_dr_matcher_layout_flags not importable from pyverbs.providers.mlx5.libmlx5. You are probably using a cpdef enum declared in a .pxd file that does not have a .py or .pyx file. <frozen importlib._bootstrap>:241: UserWarning: enum class mlx5dv_dc_type not importable from pyverbs.providers.mlx5.mlx5dv_enums. You are probably using a cpdef enum declared in a .pxd file that does not have a .py or .pyx file.

@da-woods
Copy link
Contributor Author

But now I see the following errors/warnings when I run our python tests (that uses the cython generated code):

That's definitely intentional. The change behind most of this is that when you do something like

def func():
    return SomeCpdefEnum.SomeValue

it now returns an instance of the enum class rather than just an int. However, for that to work it needs to be able to import the enum class. If it can't do that you'll get a warning and it'll just return an int.

What it's recommending you do is create a libibverbs_enums.pyx file (and compile it). It can be empty, but it'll allow the enum wrapper class to be importable.

@scoder
Copy link
Contributor

scoder commented Dec 14, 2023 via email

@EdwardSro
Copy link

What it's recommending you do is create a libibverbs_enums.pyx file (and compile it). It can be empty, but it'll allow the enum wrapper class to be importable.

But I already have a file, named enums.pyx, that is a symbolic link to libibverbs_enums.pxd and from python code the user imports from enums.pyx and it works fine. Shouldn't this behavior be kept in Cython 3?

The same goes for other warnings like libibverbs.pxd. Eventually, the user doesn't directly import from "libibverbs.pyx" but for multiple .pyx files, which themselves cimports from libibverbs.pxd...

It's either I'm missing something or the new warnings are an unwanted consequence of an underlying behavioral change in Cython.

@EdwardSro
Copy link

@da-woods @scoder is there a reason that the PR is not merged yet?

Comment on lines 5460 to 5461
if scope_name:
safe = "%s_%s" % (scope_name, safe)
Copy link
Contributor

Choose a reason for hiding this comment

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

It feels like this should use scope mangling rather than simple concatenation.

@scoder
Copy link
Contributor

scoder commented Jan 9, 2024

is there a reason that the PR is not merged yet?

Probably just the distraction by the unrelated discussion above. I left a comment.

@EdwardSro
Copy link

Will this fix be part of 3.0.9 or 3.1?

@da-woods da-woods modified the milestones: 3.0.x, 3.0.9 Feb 10, 2024
@da-woods da-woods merged commit 9b37521 into cython:master Feb 10, 2024
63 checks passed
@da-woods da-woods deleted the more-duplicate-enum-to-py-names branch February 10, 2024 11:02
da-woods added a commit that referenced this pull request Feb 10, 2024
* Fix some more duplicate enum to_py utility code names

It's possible to declare an extern enum multiple times, in a way
that makes the declaration name the same. Therefore, also prefix
declaration names with the module qualified name where possible.

* Use scope mangling
@da-woods
Copy link
Contributor Author

3.0.x commit 0185350

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

Successfully merging this pull request may close these issues.

None yet

3 participants