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

Add repr() #66

Closed
wants to merge 10 commits into from
Closed

Add repr() #66

wants to merge 10 commits into from

Conversation

mccalluc
Copy link
Contributor

@mccalluc mccalluc commented May 30, 2021

I need to get a working environment before going much further (#64, #65), but wanted to sound out this approach. The higher level goal is to just output calls to the Wrapper constructors for the values of the notebook cells, preferably the outermost Wrapper, and it will recurse to generate the string.

repr is in general not how you should do data serialization... but that's not we're doing: We want to explain what an object is (which is a good use for repr)... and incidentally, it should be executable.

In the one example I looked at in portal-ui, it didn't seem that the Wrapper instance was modified after creation... but I'm not confident that that's generally true.

Filing this as draft. If this doesn't seem horrible, I'd like to

  • get my environment working,
  • turn on doctests in this repo,
  • and add code like this to the other Wrapper classes.

Copy link
Member

@keller-mark keller-mark left a comment

Choose a reason for hiding this comment

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

Tested with this notebook:
Screen Shot 2021-06-04 at 4 54 02 PM

@keller-mark
Copy link
Member

keller-mark commented Jun 4, 2021

Just merged #67 into master, would you be able to try the following steps to set up a fresh environment:

conda deactivate
conda env remove -n vitessce-jupyter-examples
cd docs/notebooks
conda env create -f environment.yml
conda list

Check that the version of JupyterLab is greater than 3.0.0

pip install -e ../..
cd ../../js
npm install
npm run build
cd ../docs/notebooks
jupyter labextension install ../../js
jupyter lab

@mccalluc
Copy link
Contributor Author

mccalluc commented Jun 8, 2021

(I'd like #70 to be merged before I merge this one: That brings in doc test machinery, so the test here would actually be run. Keeping as draft until then.)

@mccalluc mccalluc marked this pull request as ready for review June 8, 2021 18:39
@mccalluc
Copy link
Contributor Author

mccalluc commented Jun 8, 2021

@ilan-gold -- Could this much be merged? I have not personally confirmed that it actually works for each of the types, and it wouldn't be unreasonable to ask for that before going farther.

@ilan-gold
Copy link
Collaborator

@mccalluc Do you mean in the portal? I think it would probably be good to try this out in one of the notebooks for some of them. That should be pretty straightforward and should translate to the portal. This seems fairly low-risk so even if it doesn't work as intended, the package probably won't "break"

@mccalluc
Copy link
Contributor Author

mccalluc commented Dec 6, 2021

Close in favor of #115.

@mccalluc mccalluc closed this Dec 6, 2021
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.

None yet

3 participants