manodeep / Corrfunc

⚡️⚡️⚡️Blazing fast correlation functions on the CPU.
https://corrfunc.readthedocs.io
MIT License
165 stars 50 forks source link

WIP: Use Wurlitzer to capture stderr from the C extensions and display it … #158

Closed lgarrison closed 6 years ago

lgarrison commented 6 years ago

…in Jupyter notebooks. Resolves #157.

@manodeep, do you think you'll have time to test this? It would be great to have independent verification that the command line behavior is unchanged and that error/progress messages show up in Jupyter.

Let's test on DD, then copy over to the other correlation statistics.

astropy-bot[bot] commented 6 years ago

Hi there @lgarrison :wave: - thanks for the pull request! I'm just a friendly :robot: that checks for issues related to the changelog and making sure that this pull request is milestoned and labeled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part :smiley:.

Everything looks good from my point of view! :+1:

If there are any issues with this message, please report them here.

lgarrison commented 6 years ago

The astropy bot can't seem to find our changelog. Do we need to rename it CHANGES.rst? This suggests so: https://github.com/astropy/astropy-bot/blob/master/changebot/blueprints/changelog_helpers.py#L53

manodeep commented 6 years ago

I do want to test this out. But this might be a good opportunity to setup the bot and the any style guidelines we might want to enforce. That is, make the bot enforce for C/python style

I did start a cformat branch to take care of it but totally dropped the ball. (Also, that cformat branch was not supposed to be on github either)

manodeep commented 6 years ago

Everything else looks great! Let me test out the code on my laptop, and I will report back

manodeep commented 6 years ago

I tested - works both in the command-line (with and without stdout/stderr redirects, redirects to the same file), and in a jupyter notebook

lgarrison commented 6 years ago

@manodeep I think that finishes everything I had planned for v2.2. Should we make another release? I think fixing the TypeError bug means it's probably worth it.

manodeep commented 6 years ago

Yup. I will get that new release sorted out.

manodeep commented 6 years ago

@lgarrison The README is not rendering on test.pypi.org; and all the RST linter checks are implying everything is fine. Will continue to debug

lgarrison commented 6 years ago

I see that. Weird! We barely changed the readme.

On Sat, Aug 18, 2018 at 7:28 PM Manodeep Sinha notifications@github.com wrote:

@lgarrison https://github.com/lgarrison The README is not rendering on test.pypi.org; and all the RST linter checks are implying everything is fine. Will continue to debug

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/manodeep/Corrfunc/pull/158#issuecomment-414092430, or mute the thread https://github.com/notifications/unsubscribe-auth/AFYPyy7Tc89jAOj0CncqLZcdtcOnRgYcks5uSKMGgaJpZM4U0-5s .

manodeep commented 6 years ago

Resolved by removing the raw directive. Version 2.2.0 is now on PyPI