moj-analytical-services / splink_demos

Interactive notebooks containing demonstration code of the splink library
38 stars 27 forks source link

Update demos #92

Closed RossKen closed 1 year ago

RossKen commented 1 year ago

Including:

RobinL commented 1 year ago

Is the chart rendering problem caused by the upgrade to Altair 5?

One thing I've found is that if an ipynb renders charts correctly when uploaded as a GitHub gist, then the charts also render correctly in the docs site (this is purely empirical though, and make not be 100% reliable). But it helps make for a fairly tight feedback loop in experimenting to figure out the cause of the problem.

Also happy to take a look at fixing the charts when I'm back from hol if it's my fault!

RossKen commented 1 year ago

@RobinL - the chart rendering has been an issue for a while. Waiting for the release of your Altair 5 fix later today in splink 3.9.2 then will merge this to see if it works. Thanks for the gist tip - I will test it out!

RossKen commented 1 year ago

@RobinL it looks like the charts still aren't working on Github, so if you have time to have a look at them once you are back from holiday that would be much appreciated! 🙏

RobinL commented 1 year ago

@RossKen Using this as an example.

See revisions here

File metadata differences.

Working file:

{'kernelspec': {'display_name': 'splink_demos', 'language': 'python', 'name': 'splink_demos'}, 'language_info': {'codemirror_mode': {'name': 'ipython', 'version': 3}, 'file_extension': '.py', 'mimetype': 'text/x-python', 'name': 'python', 'nbconvert_exporter': 'python', 'pygments_lexer': 'ipython3', 'version': '3.10.8'}}

Not working file:

{'kernelspec': {'display_name': 'splink_demos', 'language': 'python', 'name': 'splink_demos'}, 'language_info': {'codemirror_mode': {'name': 'ipython', 'version': 3}, 'file_extension': '.py', 'mimetype': 'text/x-python', 'name': 'python', 'nbconvert_exporter': 'python', 'pygments_lexer': 'ipython3', 'version': '3.9.2'}}

Looking at the diff in of the plain text version of the .ipynb I can see this: image

i.e. "image/png": gets added when I save in jupyter lab

Now I say that, I remember the problem: You have to save the notebook from within jupyterlab NOT in VS code For some reason VS Code doesn't save the png. This is actually independent from the update to Altair 5 - it's always been the case, but it's totally non-obvious and has caught me out historically (and then I forgot about it)

RossKen commented 1 year ago

Thanks @RobinL - that is some voodoo that I would never have worked out! I will take a crack at it and push 😊

ThomasHepworth commented 1 year ago

Some of the charts still aren't rendering - see here for an example.

Other than that, the new folder layout looks far better.

I'm happy to approve, assuming you're aware of the above issue w/ the charts.

ThomasHepworth commented 1 year ago

Oh, one more comment, actually.

We probably want to update the readme on the repo to give an overview of where everything is at a glance.

RossKen commented 1 year ago

Yea I am aware they aren't working - I would like to get the actual code updated as it is massively lagging behind and will fix the rendering separately (most likely after I have done the migration of the tutorials and examples to the main splink repo)

Given the impending migration - I am going to leave the readme for now as the purpose of this repo is going to chance so I will update once the dust has settled

RossKen commented 1 year ago

When you approve - the subsequent update of the folder structure in splink repo is good to go