Skip to content

Conversation

@PicoCentauri
Copy link
Contributor

@PicoCentauri PicoCentauri commented Jan 8, 2026

As discussed in the slack.

It make sense to also add hacks and tricks that we sometimes do to this file. @frostedoyster and @pfebrer if you have ideas just add them.


📚 Documentation preview 📚: https://metatrain--1000.org.readthedocs.build/en/1000/

@pfebrer
Copy link
Contributor

pfebrer commented Jan 8, 2026

@copilot could you check the instructions added in this PR and see if you would be able to work nicely with them

Copilot AI mentioned this pull request Jan 8, 2026
4 tasks
Copy link
Contributor

Copilot AI commented Jan 8, 2026

@pfebrer I've opened a new pull request, #1001, to work on those changes. Once the pull request is ready, I'll request review from you.

Copy link
Contributor

@pfebrer pfebrer left a comment

Choose a reason for hiding this comment

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

Thanks, I have just two comments


When reviewing code, verify:

- Imports are absolute, not relative
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not true in general, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes indeed. We rather use relative imports.

- `documentation.py`: Defines `ModelHypers` and `TrainerHypers` Pydantic classes
- `model.py`: Implements `ModelInterface` from `utils/abc.py`
- `trainer.py`: Implements `TrainerInterface` from `utils/abc.py`
- **YAML validation**: Use `OmegaConf` → Pydantic validators in `utils/pydantic.py` → `check_architecture_options()`
Copy link
Contributor

Choose a reason for hiding this comment

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

This sentence is not very clear imo, but maybe it works

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed it.

@PicoCentauri
Copy link
Contributor Author

Thanks we should also add line width as we often have linting issues

@pfebrer
Copy link
Contributor

pfebrer commented Jan 8, 2026

But this one should only be for comments or multi-line strings, which ruff doesn't format automatically. Otherwise we will get lots of suggestions that are fixable with a simple tox -e format

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