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

Header generation broken for specific dunder method #1275

Closed
Tracked by #479
Eric-Arellano opened this issue May 1, 2024 · 4 comments · Fixed by #1405
Closed
Tracked by #479

Header generation broken for specific dunder method #1275

Eric-Arellano opened this issue May 1, 2024 · 4 comments · Fixed by #1405

Comments

@Eric-Arellano
Copy link
Collaborator

### array\_\_
<Function id="array__" name="array__" signature="__array__(dtype=None)">

This should be __array__

@arnaucasau
Copy link
Collaborator

The problem with this method is that Sphinx generated the id without the double underscore at the beginning, and our script uses that id to set the header.

<dt class="sig sig-object py" id="array__">

In this case, I think it should be fixed in the qiskit repo, by adding the module's name in front of the method similar to how the rest of the methods of that page are defined. This method is special-cased by setting the module to None here, so one solution is to remove that line and line 843.

The change will also fix a cross-reference to params (line 838) that is currently broken. This will be the output of Sphinx after the fix.

Captura desde 2024-05-09 22-54-17

And this will be the new id that will help set the correct header:

<dt class="sig sig-object py" id="qiskit.circuit.array">

What do you think? I can open a PR there if you think it's a good solution. I've been trying different things like escaping the underscores, but it didn't work.

Another solution could be to change our script to use the signature for all headers, but not sure if that's feasible always (I can test it) and it will increase the complexity of the script even more.

@Eric-Arellano
Copy link
Collaborator Author

Great investigation, @arnaucasau! The output would be wrong to call it qiskit.circuit.__array__ because we're talking about a generic under method __array__ that you define on any class. We could do Gate.__array__, but better is simply __array__

What happens if you use .. py:function:: rather than py:method?

Re the broken cross-reference, can that be fixed by using the fully qualified path to Instruction.params?

@arnaucasau
Copy link
Collaborator

What happens if you use .. py:function:: rather than py:method?

Nothing changes in that case. It sets the dl class to function, but the id is the same as before 🤔

<dl class="py function">
<dt class="sig sig-object py" id="array__">

Re the broken cross-reference, can that be fixed by using the fully qualified path

Yes, we can fix params by using :attr:~qiskit.circuit.Instruction.params 👍

@Eric-Arellano
Copy link
Collaborator Author

Eric-Arellano commented May 10, 2024

Nothing changes in that case. It sets the dl class to function, but the id is the same as before 🤔

Hm, what about setting the module to builtins or __builtins__? If that doesn't work, can you get it to render as Gate.__array__ - it's fine if that includes the full module path?

@Eric-Arellano Eric-Arellano moved this to In Progress in Docs Planning May 13, 2024
@javabster javabster moved this from In Progress to In Review in Docs Planning May 13, 2024
@Eric-Arellano Eric-Arellano changed the title Header generation broken for dunder methods Header generation broken for specific dunder method May 13, 2024
github-merge-queue bot pushed a commit that referenced this issue May 17, 2024
This PR regenerates qiskit 1.1 applying a fix for #1275

Closes #1275
@github-project-automation github-project-automation bot moved this from In Review to Done in Docs Planning May 17, 2024
frankharkins pushed a commit to frankharkins/documentation that referenced this issue Jul 22, 2024
This PR regenerates qiskit 1.1 applying a fix for Qiskit#1275

Closes Qiskit#1275
@javabster javabster removed this from Docs Planning Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants