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

Remove model_connector.py #10110

Closed
daniellepintz opened this issue Oct 25, 2021 · 3 comments · Fixed by #10111
Closed

Remove model_connector.py #10110

daniellepintz opened this issue Oct 25, 2021 · 3 comments · Fixed by #10111
Assignees
Labels

Comments

@daniellepintz
Copy link
Contributor

daniellepintz commented Oct 25, 2021

Proposed refactoring or deprecation

model_connector.py has only one function copy_trainer_model_properties which is used in one place, data_connector.py. Let's move copy_trainer_model_properties directly to data_connector.py.

Motivation

We are auditing the connectors in the Architecture doc (https://docs.google.com/document/d/1xHU7-iQSpp9KJTjI3As2EM0mfNHHr37WZYpDpwLkivA/edit?pli=1#heading=h.rjokms600dhn). We can simplify things by removingmodel_connector.py and moving the function to data_connector.py.

@tchaton
Copy link
Contributor

tchaton commented Oct 25, 2021

Dear @daniellepintz,

I am good with this. @awaelchli @carmocca @Borda Good with you ?

Best,
T.C

@awaelchli
Copy link
Contributor

Yes fine with me. On top of that the code in there can probably be simplified even more than it is :)

@daniellepintz daniellepintz changed the title Deprecate model_connector.py Remove model_connector.py Oct 25, 2021
@daniellepintz
Copy link
Contributor Author

Yup, I'm doing the moving first in one PR and then we can do simplification PRs afterwards, since if we did them in the same PR it would be hard to review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants