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

Fix link types #248

Merged
merged 3 commits into from
Aug 30, 2024
Merged

Fix link types #248

merged 3 commits into from
Aug 30, 2024

Conversation

schlegelp
Copy link
Collaborator

@schlegelp schlegelp commented Aug 30, 2024

I just tried loading connectors from the VFB CATMAID:

>>> import pymaid
>>> rm = pymaid.CatmaidInstance(server="https://fafb.catmaid.virtualflybrain.org/", api_token=None)
>>> cn = pymaid.get_connectors(1875104)
>>> cn.type.value_counts()
type
unknown     856
Synaptic    118

Turns out that the link ID -> type map hard-coded in pymaid.config is different for that CATMAID instance.

This PR changes three things to fix the issue:

  1. New function, pymaid.config.get_link_types(remote_instance), that fetches the map and caches it for subsequent use.
  2. Adds a custom __hash__ method for CatmaidInstance that generates the hash based on server url, token, project ID and http user/password. This is so that changing any of the relevant properties will also change the hash and lead to re-caching of the link map (and possibly other stuff in the future).
  3. The link map contains for each connector type a name and type (plus a bunch of other variables). Previously we used the type to populate the corresponding column in the dataframe returned by get_connectors. However, on the VFB CATMAID instance we have:
    [
     {'relation_id': 14, 'name': 'Presynaptic', 'type': 'Synaptic', ...},
     {'relation_id': 17, 'name': 'Postsynaptic', 'type': 'Synaptic', ...}
    ]
    
    I.e. the type is the same for both pre- and postsynapses. With this PR we're using the name field instead.

@clbarnes I know you're out of the game technically but so am I when it comes to CATMAID. I also don't have access to your internal CATMAID instances, so can't test there. Do you see any reason why the changes suggested there might cause problems?

@clbarnes
Copy link
Collaborator

I think you're right to do this as technically it can differ between instances and hardcoding database IDs is always a risky business; this should be correct. Note that for most endpoints, the returned linked type is not the raw database ID, but a canonicalised version which puts the synaptic relations in 0, 1; gap junction in 2 etc.. I never remember which way round they are, here it is in the source https://github.com/catmaid/CATMAID/blob/2964e04e6e9772aff5d305e72c1b878030fe0e25/django/applications/catmaid/control/skeletonexport.py#L247

Good idea on the hashing for the cache!

@schlegelp schlegelp merged commit 7d169cb into master Aug 30, 2024
0 of 12 checks passed
@schlegelp schlegelp deleted the fix_link_types branch August 30, 2024 16:47
This pull request was closed.
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