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.

Sign up for GitHub

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

fixed typing that API uses #1332

Merged
merged 2 commits into from
Nov 29, 2023
Merged

fixed typing that API uses #1332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6aa136b
fixed typing that API uses
andrewelamb Nov 28, 2023
770396e
fix misplaced typing
andrewelamb Nov 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions schematic/models/metadata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,14 @@ def get_component_requirements(

# TODO: abstract validation in its own module
def validateModelManifest(
self, manifestPath: str, rootNode: str, restrict_rules: bool = False, jsonSchema: str = None, project_scope: List = None, access_token: str = None,
) -> List[str]:
self,
manifestPath: str,
rootNode: str,
restrict_rules: bool = False,
jsonSchema: Optional[str] = None,
project_scope: Optional[List] = None,
access_token: Optional[str] = None,
) -> tuple[list, list]:
"""Check if provided annotations manifest dataframe satisfies all model requirements.

Args:
Expand Down Expand Up @@ -292,13 +298,13 @@ def submit_metadata_manifest(
dataset_id: str,
manifest_record_type: str,
restrict_rules: bool,
validate_component: str = None,
access_token: str,
Copy link
Contributor

Choose a reason for hiding this comment

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

should this one be Optional[str] = None because it used to be str = None?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For access_token I removed the default of None because I was under the impression that you always needs an access token to submit since you are storing a manifest.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh right, I think I had thought this was the validation method. thanks for clarifying

Copy link
Contributor

Choose a reason for hiding this comment

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

@andrewelamb @GiaJordan We need to keep the None default, since this change actually broke the CLI submit if you require the access token. The CLI gets the access token another way from the API. I will submit another PR to update typing Optional[str]=None

validate_component: Optional[str] = None,
use_schema_label: bool = True,
hide_blanks: bool = False,
access_token: str = None,
project_scope: List = None,
table_manipulation: str = 'replace'
) -> string:
) -> str:
"""Wrap methods that are responsible for validation of manifests for a given component, and association of the
same manifest file with a specified dataset.
Args:
Expand Down
14 changes: 8 additions & 6 deletions schematic/store/synapse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,9 @@ class SynapseStorage(BaseStorage):

def __init__(
self,
token: str = None, # optional parameter retrieved from browser cookie
access_token: str = None,
project_scope: List = None,
token: Optional[str] = None, # optional parameter retrieved from browser cookie
access_token: Optional[str] = None,
project_scope: Optional[list] = None,
) -> None:
"""Initializes a SynapseStorage object.
Args:
Expand Down Expand Up @@ -302,7 +302,7 @@ def getPaginatedRestResults(self, currentUserId: str) -> Dict[str, str]:

return all_results

def getStorageProjects(self, project_scope: List = None) -> List[str]:
def getStorageProjects(self, project_scope: List = None) -> list[tuple[str, str]]:
"""Gets all storage projects the current user has access to, within the scope of the 'storageFileview' attribute.

Returns:
Expand Down Expand Up @@ -350,7 +350,7 @@ def getStorageProjects(self, project_scope: List = None) -> List[str]:

return sorted_projects_list

def getStorageDatasetsInProject(self, projectId: str) -> List[str]:
def getStorageDatasetsInProject(self, projectId: str) -> list[tuple[str, str]]:
"""Gets all datasets in folder under a given storage project that the current user has access to.

Args:
Expand Down Expand Up @@ -699,7 +699,9 @@ def _get_file_entityIds(self, dataset_files: List, only_new_files: bool = False

return files

def getProjectManifests(self, projectId: str) -> List[str]:
def getProjectManifests(
self, projectId: str
) -> list[tuple[tuple[str, str], tuple[str, str], tuple[str, str]]]:
"""Gets all metadata manifest files across all datasets in a specified project.

Returns: A list of datasets per project; metadata manifest Synapse ID for each dataset; and the corresponding schema component of the manifest
Expand Down
Loading