Skip to content
This repository has been archived by the owner on Sep 13, 2023. It is now read-only.

Get rid of mlem dir #395

Merged
merged 7 commits into from
Oct 12, 2022
Merged

Get rid of mlem dir #395

merged 7 commits into from
Oct 12, 2022

Conversation

mike0sv
Copy link
Contributor

@mike0sv mike0sv commented Sep 2, 2022

closes #381

@mike0sv mike0sv requested a review from a team September 2, 2022 00:03
@mike0sv mike0sv self-assigned this Sep 2, 2022
@mike0sv mike0sv temporarily deployed to internal September 2, 2022 00:03 Inactive
@mike0sv mike0sv temporarily deployed to internal October 7, 2022 11:34 Inactive
@mike0sv mike0sv changed the base branch from main to release/0.3.0 October 7, 2022 11:34
@mike0sv mike0sv temporarily deployed to internal October 7, 2022 12:53 Inactive
@mike0sv mike0sv temporarily deployed to internal October 7, 2022 13:13 Inactive
@codecov
Copy link

codecov bot commented Oct 7, 2022

Codecov Report

Base: 87.62% // Head: 87.60% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (7eb35c9) compared to base (d60f3cf).
Patch coverage: 75.10% of modified lines in pull request are covered.

Additional details and impacted files
@@                Coverage Diff                @@
##           release/0.3.0     #395      +/-   ##
=================================================
- Coverage          87.62%   87.60%   -0.02%     
=================================================
  Files                 94       94              
  Lines               7847     7714     -133     
=================================================
- Hits                6876     6758     -118     
+ Misses               971      956      -15     
Impacted Files Coverage Δ
mlem/api/__init__.py 100.00% <ø> (ø)
mlem/cli/apply.py 94.23% <ø> (ø)
mlem/cli/build.py 100.00% <ø> (ø)
mlem/cli/checkenv.py 100.00% <ø> (ø)
mlem/cli/clone.py 100.00% <ø> (ø)
mlem/cli/dev.py 50.00% <ø> (ø)
mlem/cli/import_object.py 100.00% <ø> (ø)
mlem/cli/init.py 100.00% <ø> (ø)
mlem/cli/link.py 100.00% <ø> (ø)
mlem/cli/serve.py 89.47% <ø> (ø)
... and 66 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

"""Creates .mlem directory in `path`"""
path = posixpath.join(path, MLEM_DIR)
"""Creates mlem config in `path`"""
path = posixpath.join(path, MLEM_CONFIG_FILE_NAME)
Copy link
Contributor

@aguschin aguschin Oct 7, 2022

Choose a reason for hiding this comment

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

I know the file is called .mlem.yaml. But for GTO it's .gto. Should we rename it to .gto.yaml in GTO, or should we rename .mlem.yaml to .mlem? We shouldn't confuse users with different approaches. Did we had same files in other tools maybe? cc @shcheklein

Copy link
Contributor

Choose a reason for hiding this comment

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

Also if users have .mlem dir now, it's not possible to create .mlem file. So renaming config file will force them to move everything outside of .mlem dir and change their workflows maybe.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mike0sv if everything else is good, let's merge this so we won't get a PR queue again? And then discuss it separately.

Copy link
Contributor

Choose a reason for hiding this comment

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

my 2 cents - if the file is indeed a yaml, gto.yaml > [something.]gto
extensions are for formats :)
as long as gto doesn't invent a new file format/structure but respects yaml, that's the least surprising thing to do

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, then it's decided I guess. Let's keep it here, and I'll update it in GTO as well some time later.

Copy link
Contributor

@aguschin aguschin left a comment

Choose a reason for hiding this comment

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

LGTM! close #381

@aguschin aguschin merged commit b7aeac2 into release/0.3.0 Oct 12, 2022
@aguschin aguschin deleted the feature/no-mlem-dir branch October 12, 2022 08:54
@aguschin aguschin mentioned this pull request Nov 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

MLEM index and .mlem/ folder
3 participants