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

Don't use IFrames in notebooks under vscode, add support for altair plot in vscode #57

Merged
merged 7 commits into from
Jul 29, 2024

Conversation

gbowlin
Copy link
Contributor

@gbowlin gbowlin commented Jul 25, 2024

Overview

Closes #53

Description of changes

Adds the ability to detect the notebook host, so that we can change display behavior based on which host we are in. This is useful when dealing with IFrames in particular, because vscode does not allow them to render local content.

Since the Altair plot for Aequitas doesn't load well as HTML, we have used the host detection ability to choose whether or not we want to save the plot to HTML/IFrame or just display the plot as is.

Testing Instructions

  • make sure you can run the default notebook in vscode and see the fairness control and outcomes
  • make sure you can run the default notebook via jupyter lab and see the fairness control and outcomes

Author Checklist

  • Linting passes; run early with pre-commit hook.
  • Tests added for new code and issue being fixed.
  • Added type annotations and full numpy-style docstrings for new methods.
  • Draft your news fragment in new changelog/ISSUE.TYPE.rst files; see changelog/README.md.

@gbowlin gbowlin changed the title Donadd vscode support for altair plot Don't use IFrames in notebooks under vscode, add support for altair plot in vscode Jul 25, 2024
@gbowlin gbowlin self-assigned this Jul 25, 2024
@gbowlin gbowlin added the bug Something isn't working label Jul 25, 2024
src/seismometer/core/nbhost.py Outdated Show resolved Hide resolved
src/seismometer/core/nbhost.py Outdated Show resolved Hide resolved
src/seismometer/core/nbhost.py Outdated Show resolved Hide resolved
@gbowlin gbowlin requested a review from diehlbw July 29, 2024 14:32
src/seismometer/_api.py Show resolved Hide resolved
src/seismometer/report/auditing.py Show resolved Hide resolved
tests/core/test_nbhost.py Outdated Show resolved Hide resolved
@gbowlin gbowlin requested a review from diehlbw July 29, 2024 17:41
diehlbw
diehlbw previously approved these changes Jul 29, 2024
@diehlbw diehlbw mentioned this pull request Jul 29, 2024
9 tasks
@gbowlin gbowlin removed the request for review from MahmoodEtedadi July 29, 2024 18:50
@gbowlin gbowlin marked this pull request as ready for review July 29, 2024 18:58
diehlbw
diehlbw previously approved these changes Jul 29, 2024
@diehlbw diehlbw merged commit 3d8c13e into epic-open-source:main Jul 29, 2024
14 checks passed
diehlbw added a commit to diehlbw/seismometer that referenced this pull request Jul 29, 2024
diehlbw added a commit to diehlbw/seismometer that referenced this pull request Jul 31, 2024
@gbowlin gbowlin deleted the dont_use_iframes_in_vscode branch August 7, 2024 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants