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

API generation script - Fix loadImages function #665

Merged
merged 2 commits into from
Jan 22, 2024

Conversation

arnaucasau
Copy link
Collaborator

@arnaucasau arnaucasau commented Jan 22, 2024

The processHtml.ts script assumes that we always have an src attribute in the img tags, and even though this is true for the latest API versions, for Qiskit historical versions, we can find cases like the following example:

Qiskit v0.41 release_notes.html line 23360

<div class="animation">
  <img id="_anim_imga6548212aea7457ca5d2f940fabeaea3">
  <div class="anim-controls">
    <input id="_anim_slidera6548212aea7457ca5d2f940fabeaea3" type="range" class="anim-slider"

This PR adds an early return to the loadImage function to skip the images without a src attribute given that we can't find the image in the _images folder of the GitHub artifact.

@arnaucasau arnaucasau changed the title API generation - Fix loadImages function API generation script - Fix loadImages function Jan 22, 2024
Copy link
Member

@frankharkins frankharkins left a comment

Choose a reason for hiding this comment

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

Nice fix! This logic looks good but maybe using filter would be neater?

Copy link
Collaborator

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

+1 to .filter().map() being more idiomatic. Good find!

@arnaucasau
Copy link
Collaborator Author

Good idea @frankharkins! Thank you both for the review

@arnaucasau arnaucasau added this pull request to the merge queue Jan 22, 2024
Merged via the queue into Qiskit:main with commit 488f833 Jan 22, 2024
2 checks passed
@arnaucasau arnaucasau deleted the fix-load-images branch January 22, 2024 14:05
frankharkins pushed a commit to frankharkins/documentation that referenced this pull request Jul 22, 2024
The `processHtml.ts` script assumes that we always have an `src`
attribute in the `img` tags, and even though this is true for the latest
API versions, for Qiskit historical versions, we can find cases like the
following example:

**Qiskit v0.41 release_notes.html line 23360**
```html
<div class="animation">
  <img id="_anim_imga6548212aea7457ca5d2f940fabeaea3">
  <div class="anim-controls">
    <input id="_anim_slidera6548212aea7457ca5d2f940fabeaea3" type="range" class="anim-slider"
```

This PR adds an early return to the `loadImage` function to skip the
images without a `src` attribute given that we can't find the image in
the `_images` folder of the GitHub artifact.
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.

3 participants