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

Revise path logic and move models and data to top-level #30

Merged
merged 11 commits into from
Jun 17, 2024

Conversation

mdekstrand
Copy link
Contributor

@mdekstrand mdekstrand commented Jun 14, 2024

This updates the path-location logic and rearranges the repository structure to be more logical and reduce wasted space during Docker image build:

  • establishes top-level src, data, models directories for source code, input data, and model checkpoints, respectively.
  • adds model_file_path function to locate a model file, checking in a POPROX_MODELS environment variable, the project root, and the Conda environment prefix, in that order. This will enable POPROX model data to be stored in a data location during deployment.
  • restores project_root and makes it search based on the current working directory, looking for pyproject.toml, so that it can work seamlessly regardless of whether and how poprox_recommenders has been installed.

It also adds tests for the paths module, and depends on #31 and the docker tests to make sure none of this breaks our code in a way that can be detected at test time.

@mdekstrand mdekstrand force-pushed the mdekstrand/refactor-model-locations branch from 38c025d to 104ca26 Compare June 14, 2024 22:42
@mdekstrand mdekstrand force-pushed the mdekstrand/refactor-model-locations branch from d02a6d6 to 022a15f Compare June 17, 2024 14:31
@mdekstrand mdekstrand marked this pull request as ready for review June 17, 2024 14:35
@@ -36,7 +36,7 @@ def recsys_metric(recommendations, row_index, news_struuid_ID):
# use the url of Article
impressions_truth = (
pd.read_table(
src_dir() / "data" / "test_mind_large" / "behaviors.tsv",
f"{project_root()}/data/test_mind_large/behaviors.tsv",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's keep the pathlib style here

@mdekstrand mdekstrand merged commit 9ed9f3b into main Jun 17, 2024
4 checks passed
@mdekstrand mdekstrand deleted the mdekstrand/refactor-model-locations branch June 17, 2024 15:03
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.

2 participants