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

POC: exposing the watermark function outside Jupyter #46

Merged
merged 7 commits into from Feb 18, 2021

Conversation

twolodzko
Copy link

@twolodzko twolodzko commented Jan 3, 2019

This is a proof-of-concept PR.

I exposed the watermark() function, so that it also works outside Jupyter. Now the magic %watermark calls watermark() function. So now this works both outside and from Jupyter as magic function.

Changes in the behavior: new function get_imported_packages() is introduced, it prints more of the buildin packages then the initial code. At this moment I have no idea how this could be fixed.

The code obviously needs cleaning-up.

@pep8speaks
Copy link

pep8speaks commented Jan 3, 2019

Hello @twolodzko! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-02-18 16:35:35 UTC

@rasbt
Copy link
Owner

rasbt commented Feb 17, 2021

I am actually not sure what happened to this PR. Was/is this still work in progress? Would be happy to revisit.

@twolodzko
Copy link
Author

@rasbt I also already forgot about this one 😆 No work is done on this. If it works, it works. If you want me to change something, feel free to comment.

@rasbt
Copy link
Owner

rasbt commented Feb 17, 2021

Thanks for the PR and note! I may have ignored this because I wasn't sure that it was ready and then forgot. Will put it onto my todo list again. Thanks!

@rasbt
Copy link
Owner

rasbt commented Feb 18, 2021

Alright! Just took this existing PR and patched it into the recent 2.1 version. It was a bit messy because we had a major refactoring back then, but I hope I didn't screw it up. There was also a consequent merge conflict, but on my end, now things seem to work :).

Maybe @twolodzko have a look if this looks good to merge. Would also appreciate a second pair of eyes, @jamesmyatt . Maybe after merging this we could finally add unit tests.

@twolodzko
Copy link
Author

I have no comments, LGTM

@rasbt
Copy link
Owner

rasbt commented Feb 18, 2021

Great! I saw I had some installation cluttering so more fixes were required, but it should be all good now. Will merge and then replace Travis with Appveyor to run the first unittest. (Travis requires credits now, which are unfortunately consumed by other github repos so Travis would not be reliable here)

@rasbt rasbt merged commit 8740916 into rasbt:master Feb 18, 2021
Hugovdberg added a commit to Hugovdberg/watermark that referenced this pull request Dec 8, 2021
Build upon rasbt#46 to allow injecting globals when used outside IPython.
Hugovdberg added a commit to Hugovdberg/watermark that referenced this pull request Dec 8, 2021
Build upon rasbt#46 to allow injecting globals when used outside IPython.
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