Skip to content

Fix UniProt alias error handling - #296

Open
pablosanchezizquierdo wants to merge 3 commits into
develfrom
fix294_uniprot_alias_error_handling
Open

Fix UniProt alias error handling#296
pablosanchezizquierdo wants to merge 3 commits into
develfrom
fix294_uniprot_alias_error_handling

Conversation

@pablosanchezizquierdo

Copy link
Copy Markdown
Collaborator

fix #294

This PR improves the error handling in UniProt._get_aliases().

Changes

  • return a complete alias dictionary instead of returning None when an alias cannot be resolved
  • return None for unresolved aliases while continuing to process the remaining requested aliases
  • handle invalid responses from the UniProt mapping service more robustly

@mtiberti mtiberti left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is fine, but I think we should also handle this case in the calling function.

What happens if the conversion fails and this is None?

I think we should check and raise an Exception in the calling function e.g. for ValueError unless we have a way around it

Comment thread cancermuts/datasources.py Outdated
if t_keyword is not None:
self.log.info('using extracted keyword %s to parse results' % t_keyword)
out[t] = results['to'][t_keyword]
out[t] = results['to'].get(t_keyword)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why this change?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

it was to avoid KeyError but if we handle the error in get_sequence I can revert it

Comment thread cancermuts/datasources.py
Comment thread cancermuts/datasources.py
if upid is None:
this_upid = self._get_aliases(this_upac, ['UniProtKB_uniProtkbId'])['UniProtKB_uniProtkbId']
if this_upid is None:
raise ValueError(f"Could not resolve UniProt primary accession for {this_upac}")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this still primary accession?

this is different from the other two calls

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