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

Test error with new versioning #471

Open
frostedoyster opened this issue Feb 1, 2025 · 9 comments
Open

Test error with new versioning #471

frostedoyster opened this issue Feb 1, 2025 · 9 comments
Labels
Bug Something isn't working Priority: Medium Important issues to address after high priority.

Comments

@frostedoyster
Copy link
Collaborator

Running the tests locally with python -m pytest errors due to the fact that _version is not present (I believe pytest tests from the metatrain folder and not its installation, which would instead contain the module)
@PicoCentauri
@Luthaf

@frostedoyster frostedoyster added Bug Something isn't working Priority: Medium Important issues to address after high priority. labels Feb 1, 2025
@PicoCentauri
Copy link
Contributor

Hmmm, this is weird. If you haven't installed the package pytest should use (an old) installed version of your enviroment, which should not have the _version.py. And once you installed it the file should be present.

May I ask why are you using pytest directly and not via tox in an isolated environment?

@frostedoyster
Copy link
Collaborator Author

I like using pytest directly because it's much faster (no new environment to create) and because it gives me control over the environment that I'm running the tests in

I don't know if it's expected, but, as long as I was installing with --no-build-isolation --no-deps or --no-build-isolation I was getting the missing _version errors, but then I installed once without the flags and everything is working well. I have no clue what's going on

@PicoCentauri
Copy link
Contributor

I assume when you use these flags the build process is not triggered fully and setuptools-scm is not triggered to create the version file.

I thinl there is not mhuch we can do. Basically these extra options are dangerous and you have to know what you are doing 😝

@frostedoyster
Copy link
Collaborator Author

I see that a _version.py was created in my directory (and git wants to commit it, we should probably add it to gitignore)

@PicoCentauri
Copy link
Contributor

It is...

@Luthaf
Copy link
Member

Luthaf commented Feb 3, 2025

I don't know if it's expected, but, as long as I was installing with --no-build-isolation --no-deps or --no-build-isolation I was getting the missing _version errors, but then I installed once without the flags and everything is working well. I have no clue what's going on

You really should not need --no-build-isolation for this project, since the point of the flag for us is to (a) prevent installing torch & co in a separate virtualenv and (b) build against an already installed torch version. Since metatrain does not depend on torch at build time, this does nothing.

@Luthaf
Copy link
Member

Luthaf commented Feb 3, 2025

Overall, I think this is not an officially supported workflow, only tox is. If tox is too slow for you we can try to make it faster, but it should not re-create environments after the first time.

@frostedoyster
Copy link
Collaborator Author

--no-build-isolation was for the soap-bpnn extra dependency
I tried to install again with --no-build-isolation and it didn't work. The first time that you install without the flag, then it starts working. IMO, in general, we should support --no-build-isolation so that more optimized and customized versions can be used on HPC platforms

@Luthaf
Copy link
Member

Luthaf commented Feb 6, 2025

We don't need --no-build-isolation for metatrain itself. If someone wants to play with this, I would suggest installing the dependencies manually with --no-build-isolation, and then pip install metatrain --no-deps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Priority: Medium Important issues to address after high priority.
Projects
None yet
Development

No branches or pull requests

3 participants