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

BAI-1253 Client Datacard Class #1272

Merged
merged 13 commits into from
May 21, 2024
Merged

BAI-1253 Client Datacard Class #1272

merged 13 commits into from
May 21, 2024

Conversation

bw27492
Copy link
Member

@bw27492 bw27492 commented May 14, 2024

  • Added new parent 'Entry' class for both models and datacards.
  • Moved non-unique methods from existing model class to new parent class.
  • Created new datacard class

@bw27492 bw27492 requested review from GB27247 and a3957273 May 14, 2024 15:48
@bw27492 bw27492 changed the title BAI-1253 Client Dataset Class BAI-1253 Client Datacard Class May 14, 2024
name=res["name"],
description=res["description"],
)
datacard._Entry__unpack(res)
Copy link
Member

Choose a reason for hiding this comment

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

Python lacks real access modifiers, although through name mangling will add _Class to any method with a double underscore these are considered private and shouldn't be accessed by any child classes. Instead it is convention to use a single underscore for protected methods(methods to be accessed by child classes).

Copy link
Member Author

Choose a reason for hiding this comment

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

Done


@property
def data_card(self):
return self.card
Copy link
Member

Choose a reason for hiding this comment

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

I would make self.card a protected attribute since you're effectively aliasing it here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done


@property
def data_card_version(self):
return self.card_version
Copy link
Member

Choose a reason for hiding this comment

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

Same with card_version

Copy link
Member Author

Choose a reason for hiding this comment

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

Done


@property
def data_card_schema(self):
return self.card_schema
Copy link
Member

Choose a reason for hiding this comment

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

Same with card_schema

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Comment on lines 39 to 97
@classmethod
def create(
cls,
client: Client,
name: str,
description: str,
team_id: str,
visibility: ModelVisibility | None = None,
) -> Datacard:
"""Build a datacard from Bailo and upload it.

:param client: A client object used to interact with Bailo
:param name: Name of datacard
:param description: Description of datacard
:param team_id: A unique team ID
:param visibility: Visibility of datacard, using ModelVisibility enum (e.g Public or Private), defaults to None
:return: Datacard object
"""
res = client.post_model(
name=name, kind=EntryKind.DATACARD, description=description, team_id=team_id, visibility=visibility
)
datacard = cls(
client=client,
datacard_id=res["model"]["id"],
name=name,
description=description,
visibility=visibility,
)

datacard._Entry__unpack(res["model"])

return datacard

@classmethod
def from_id(cls, client: Client, datacard_id: str) -> Datacard:
"""Return an existing datacard from Bailo.

:param client: A client object used to interact with Bailo
:param datacard_id: A unique datacard ID
:return: A datacard object
"""
res = client.get_model(model_id=datacard_id)["model"]
datacard = cls(
client=client,
datacard_id=datacard_id,
name=res["name"],
description=res["description"],
)
datacard._Entry__unpack(res)

datacard.get_card_latest()

return datacard
Copy link
Member

Choose a reason for hiding this comment

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

Can use these as protected class methods in the parent class.

Comment on lines 72 to 97
@classmethod
def from_id(cls, client: Client, datacard_id: str) -> Datacard:
"""Return an existing datacard from Bailo.

:param client: A client object used to interact with Bailo
:param datacard_id: A unique datacard ID
:return: A datacard object
"""
res = client.get_model(model_id=datacard_id)["model"]
datacard = cls(
client=client,
datacard_id=datacard_id,
name=res["name"],
description=res["description"],
)
datacard._Entry__unpack(res)

datacard.get_card_latest()

return datacard
Copy link
Member

Choose a reason for hiding this comment

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

What's stopping me from pulling down a Model as a Datacard here. There isn't anything differentiating them apart from their kind.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@a3957273 a3957273 force-pushed the BAI-1253-client-dataset-class branch from 10c3020 to eb0e7c3 Compare May 21, 2024 10:52
@bw27492 bw27492 force-pushed the BAI-1253-client-dataset-class branch from eb0e7c3 to 10c3020 Compare May 21, 2024 14:28
@bw27492 bw27492 merged commit cfcc0ea into main May 21, 2024
28 checks passed
@bw27492 bw27492 deleted the BAI-1253-client-dataset-class branch May 21, 2024 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants