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

Document gotchas when interoperation with python #804

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

jw3126
Copy link

@jw3126 jw3126 commented Jan 14, 2021

docs/src/h5py.md Outdated

The whole example as a Julia executable script:
```julia
using PyCall
Copy link
Author

@jw3126 jw3126 Jan 14, 2021

Choose a reason for hiding this comment

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

We could add PyCall as a test dependency and turn this into a doctest. However it might be fragile since we need to install h5py.

Copy link
Member

Choose a reason for hiding this comment

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

Is possible to use Conda.jl to install the hdf5 library in order to turn these to doctests. I'm not opposed to adding a PyCall test dependency. In general, it does seem to be a good idea to ensure inter-op.

Copy link
Author

Choose a reason for hiding this comment

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

We could, but this introduces further headaches. If you run tests locally using e.g. system python things may fail.

Copy link
Author

Choose a reason for hiding this comment

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

I added this anyway.

docs/src/h5py.md Outdated Show resolved Hide resolved
docs/src/h5py.md Outdated Show resolved Hide resolved
@jw3126
Copy link
Author

jw3126 commented Jan 15, 2021

BTW is it intended, that CI passes, despite the doctest fail?

@musm
Copy link
Member

musm commented Jan 16, 2021

BTW is it intended, that CI passes, despite the doctest fail?

It shouldn't, not sure why it passed yet.

@musm
Copy link
Member

musm commented Jan 16, 2021

I'm trying to decide if we should add Literate as a test dependency. What was the specific reason before the last commit that spawned you to switch over to use Literate?

@jw3126
Copy link
Author

jw3126 commented Jan 18, 2021

I'm trying to decide if we should add Literate as a test dependency. What was the specific reason before the last commit that spawned you to switch over to use Literate?

The only reason is, that I find it easier to develop a .jl document, which is executable Julia code, instead of a .md document that I can only run doctests on. I removed the Literate dependency.

@jw3126
Copy link
Author

jw3126 commented Jan 25, 2021

@musm
Copy link
Member

musm commented Oct 18, 2021

Let's try running thing again here

@musm musm closed this Oct 18, 2021
@musm musm reopened this Oct 18, 2021
@mkitti mkitti closed this Mar 25, 2022
@mkitti mkitti reopened this Mar 25, 2022
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.

3 participants