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

Update NbdevLookup to support import aliases and improve docstrings #1471

Merged
merged 2 commits into from
Nov 21, 2024

Conversation

ncoop57
Copy link

@ncoop57 ncoop57 commented Nov 20, 2024

This PR adds support to us NbdevLookup on import aliases such as import numpy as np. For example, you can now do the following:

import numpy as np
NbdevLookup(ns=globals()).linkify('this is an aliased import link `np.array`')
'this is an aliased import link [`np.array`](https://numpy.org/doc/stable/reference/generated/numpy.array.html#numpy.array)'

The way this works, is that an alias mapping is created in NbdevLookup:

self.aliases = {n:o.__name__ for n,o in (ns or {}).items() if isinstance(o, ModuleType)}

This maps the alias to its true name. We then use this in the get item function when necessary:

def __getitem__(self, s): 
        if '.' in s:
            pre,post = s.split('.', 1)
            if pre in self.aliases: s = f"{self.aliases[pre]}.{post}"
        return self.syms.get(s, None)

However, it required removing lru_cache(None) as a class level decorator since the dictionary for the namespace used to create the aliases of the imports and their true symbol names is not hashable. A work around would be to not do it on the class level and just decorate the functions that need it, but wanted to open up the PR to start a discussion on this as a direction before investing more time into it.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@ncoop57
Copy link
Author

ncoop57 commented Nov 20, 2024

@jph00

@ncoop57
Copy link
Author

ncoop57 commented Nov 21, 2024

I added a commit that separates out the building of the lookup table from the init function of nbdev lookup class, as discussed with @jph00. Also added some test cases that evaluate whether or not the caching is actually working. I think it's ready for primetime!

@jph00
Copy link
Contributor

jph00 commented Nov 21, 2024

Super great @ncoop57 ! :D

@jph00 jph00 merged commit 05deba5 into AnswerDotAI:main Nov 21, 2024
10 checks passed
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