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

Missing __version__ in generated python module #16

Closed
MuellerSeb opened this issue Nov 18, 2021 · 5 comments
Closed

Missing __version__ in generated python module #16

MuellerSeb opened this issue Nov 18, 2021 · 5 comments

Comments

@MuellerSeb
Copy link
Member

I noticed, that maturin doesn't add a __version__ string to the generated module.

One possibility is given here: PyO3/maturin#100 (comment)

    m.add("__version__", env!("CARGO_PKG_VERSION"))?;

See also here: https://github.com/PyO3/pyo3-built/

@MuellerSeb
Copy link
Member Author

Also https://github.com/PyO3/pyo3-built/ would be a good thing to include, I guess.
What do you think, @LSchueler @adamreichold ?

@adamreichold
Copy link
Contributor

I am admittedly somewhat sceptical. Having inline provenance data is certainly nice, but do we have a specific use case in mind? Adding a build script does have a cost w.r.t. build duration and additional failure modes. (But then our build configuration uses LTO and a single codegen unit, so build times already are horrible.)

Personally, I would start with the minimal version info as suggested in the top comment.

@MuellerSeb
Copy link
Member Author

Ok. Then I would suggest adding version info and making a release so we can release gstools 1.3.4

@LSchueler
Copy link
Member

Hmm, that's a tough one. Reproducibility is maybe even more important in scientific software than in other software and detailed environment info about versions, backend, ... definitely helps in that regard. But even longer build times do sound horrible.
So yeah, maybe start out with only adding the version info and afterwards we could experiment with the pyo3-built.

@LSchueler
Copy link
Member

I'm closing this for now due to PR #17.

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

No branches or pull requests

3 participants