Fix wrong name assignment in parser #42
Conversation
|
The test is working. I'm also deleting |
tests/cli/test_train_model.py
Outdated
| @@ -26,6 +27,33 @@ def test_train(monkeypatch, tmp_path, output): | |||
| assert Path(output).is_file() | |||
|
|
|||
|
|
|||
| @pytest.mark.parametrize("output", [None, "mymodel.pt"]) | |||
| def test_train_explicit_validation_test(monkeypatch, tmp_path, output): | |||
There was a problem hiding this comment.
Thanks for adding the test. We should also test a combination of these. Like both as a float each one as a float and both as files.
There was a problem hiding this comment.
Hmm I don't know if like that we add three more files here. Maybe keep the explicit and load this one, overwrite them and save them to the custom directory with the float options.
tests/cli/test_train_model.py
Outdated
| @@ -26,6 +27,33 @@ def test_train(monkeypatch, tmp_path, output): | |||
| assert Path(output).is_file() | |||
|
|
|||
|
|
|||
| @pytest.mark.parametrize("output", [None, "mymodel.pt"]) | |||
| def test_train_explicit_validation_test(monkeypatch, tmp_path, output): | |||
There was a problem hiding this comment.
Hmm I don't know if like that we add three more files here. Maybe keep the explicit and load this one, overwrite them and save them to the custom directory with the float options.
tests/cli/test_train_model.py
Outdated
| command = ["metatensor-models", "train", "options_explicit_valid_test.yaml"] | ||
| command = ["metatensor-models", "train", "options_explicit_valid.yaml"] | ||
| command = ["metatensor-models", "train", "options_explicit_test.yaml"] |
There was a problem hiding this comment.
I would rather like this as a pytest.mark.parametrize. These jobs creates files and this may interfere with each other.
PicoCentauri
force-pushed
the
parser-fixes
branch
from
4a7651c
to
0115dcf
Compare
|
Should be now ready @frostedoyster. |
|
Thanks a lot @PicoCentauri!! |
Just missing a test. To be merged soon
📚 Documentation preview 📚: https://metatensor-models--42.org.readthedocs.build/en/42/