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

return ls as internal API #477

Merged
merged 2 commits into from
Nov 21, 2022
Merged

return ls as internal API #477

merged 2 commits into from
Nov 21, 2022

Conversation

mike0sv
Copy link
Contributor

@mike0sv mike0sv commented Nov 8, 2022

close #478

@mike0sv mike0sv requested a review from a team November 8, 2022 14:41
@mike0sv mike0sv self-assigned this Nov 8, 2022
@aguschin
Copy link
Contributor

Wow, do you have any idea why CI didn't run? 🤔

@mike0sv mike0sv changed the base branch from main to ci/add-python-3.11 November 10, 2022 12:06
@mike0sv mike0sv changed the base branch from ci/add-python-3.11 to main November 10, 2022 12:06
@mike0sv
Copy link
Contributor Author

mike0sv commented Nov 10, 2022

No idea

@mike0sv mike0sv temporarily deployed to internal November 10, 2022 12:08 Inactive
@codecov
Copy link

codecov bot commented Nov 10, 2022

Codecov Report

Base: 60.01% // Head: 87.31% // Increases project coverage by +27.29% 🎉

Coverage data is based on head (7594750) compared to base (6b511fd).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #477       +/-   ##
===========================================
+ Coverage   60.01%   87.31%   +27.29%     
===========================================
  Files          95       96        +1     
  Lines        8135     8141        +6     
===========================================
+ Hits         4882     7108     +2226     
+ Misses       3253     1033     -2220     
Impacted Files Coverage Δ
mlem/cli/main.py 84.58% <ø> (+27.66%) ⬆️
mlem/contrib/callable.py 98.27% <100.00%> (+61.83%) ⬆️
mlem/contrib/fastapi.py 94.91% <100.00%> (+53.05%) ⬆️
mlem/core/metadata.py 97.05% <100.00%> (+38.56%) ⬆️
mlem/contrib/heroku/utils.py 24.59% <0.00%> (-55.74%) ⬇️
mlem/contrib/heroku/build.py 58.06% <0.00%> (-32.26%) ⬇️
mlem/contrib/heroku/meta.py 56.62% <0.00%> (-30.13%) ⬇️
mlem/contrib/pip/setup.py.j2 100.00% <0.00%> (ø)
mlem/ext.py 88.28% <0.00%> (+1.80%) ⬆️
mlem/contrib/docker/context.py 85.48% <0.00%> (+2.41%) ⬆️
... and 68 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.

@mike0sv mike0sv temporarily deployed to internal November 10, 2022 13:03 Inactive
and parent != MlemObject
and issubclass(parent, MlemObject)
):
type_ = parent
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be folded into a MlemObject method meta.get_mlem_type() that handles links/non links?
or does this not generalize well enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dont recall any other places with such logic.
So we can have

  1. a link - we put it into whatever it links to
  2. a non-polymorphic object, which goes to it's type bucket
  3. a polymorphic object, which goes to it's parent type bucket

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand, just feels like messing with the classes' internals. So this type resolution is only specific to the ls implementation today?

Copy link
Contributor

Choose a reason for hiding this comment

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

@mike0sv? :)

Copy link
Contributor

Choose a reason for hiding this comment

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

So this type resolution is only specific to the ls implementation today?

Yes.

@aguschin aguschin added the api MLEM Python API label Nov 21, 2022
@aguschin aguschin merged commit dadbd11 into main Nov 21, 2022
@aguschin aguschin deleted the feature/return-ls branch November 21, 2022 09:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api MLEM Python API
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Re-implement mlem.api.ls
3 participants