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

Rename folders #1478

Merged
merged 19 commits into from
Jul 16, 2021
Merged

Rename folders #1478

merged 19 commits into from
Jul 16, 2021

Conversation

miguelgfierro
Copy link
Collaborator

Description

Related Issues

#1390

Checklist:

  • I have followed the contribution guidelines and code style for this project.
  • I have added tests covering my contributions.
  • I have updated the documentation accordingly.
  • This PR is being made to staging branch and not to main branch.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@wutaomsft
Copy link
Collaborator

This work will trigger a new major version, correct? @miguelgfierro

@anargyri
Copy link
Collaborator

anargyri commented Jul 16, 2021

Wrong renames:

 reco_utils/dataset/__init__.py → reco_utils/models/__init__.py 

 reco_utils/recommender/__init__.py → reco_utils/models/cornac/__init__.py 

reco_utils/recommender/cornac/__init__.py → ...tils/models/deeprec/DataModel/__init__.py 

...recommender/deeprec/DataModel/__init__.py → reco_utils/models/deeprec/__init__.py 

reco_utils/recommender/deeprec/__init__.py → reco_utils/models/deeprec/io/__init__.py 

..._utils/recommender/deeprec/io/__init__.py → reco_utils/models/deeprec/models/__init__.py 

...ender/deeprec/models/graphrec/__init__.py → ...els/deeprec/models/sequential/__init__.py 

 ...der/deeprec/models/sequential/__init__.py → reco_utils/models/fastai/__init__.py 

 reco_utils/recommender/fastai/__init__.py → reco_utils/models/geoimc/__init__.py 

Etc. Maybe they cancel out. Good to double check whether all the empty init files are there.

@miguelgfierro
Copy link
Collaborator Author

about this @anargyri:

Wrong renames:
reco_utils/dataset/init.py → reco_utils/models/init.py
reco_utils/recommender/init.py → reco_utils/models/cornac/init.py
reco_utils/recommender/cornac/init.py → ...tils/models/deeprec/DataModel/init.py
...recommender/deeprec/DataModel/init.py → reco_utils/models/deeprec/init.py
reco_utils/recommender/deeprec/init.py → reco_utils/models/deeprec/io/init.py

I think the changes are ok, when I did git mv, I think git can't assign between init files because they are the exact same empty file

@miguelgfierro
Copy link
Collaborator Author

miguelgfierro commented Jul 16, 2021

This work will trigger a new major version, correct? @miguelgfierro

@wutaomsft we could do a new release if people want. Actually, now that we are changing paths, maybe we should consider other changes:

  1. Do we want to change reco_utils to recommenders?
  2. Do we want to change the folder of hybrid and move it to either CBF or CF?

I'll start a chat on teams about this

@miguelgfierro miguelgfierro merged commit 79f8ced into staging Jul 16, 2021
@miguelgfierro miguelgfierro deleted the miguel/rename_folders branch July 16, 2021 21:15
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