fastai / nbdev

Create delightful software with Jupyter Notebooks
https://nbdev.fast.ai/
Apache License 2.0
4.9k stars 487 forks source link

Fix sync for nested libraries #1394

Closed scwe closed 3 days ago

scwe commented 9 months ago

A fix for https://github.com/fastai/nbdev/issues/1393. In essence this fixes it by using the complete library path for importing the modidx file rather than just grabbing the lowest level subdirectory from the path.

I'm not entirely sure this is the right fix for the problem, but in the event that it is hopefully it makes your lives very easy.

review-notebook-app[bot] commented 9 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

scwe commented 4 months ago

Hey @jph00 and/or @hamelsmu :wave: I'm not sure who best to ping on this to get some eyes on this so we can start using nbdev_update in our project.

jph00 commented 1 month ago

Sorry I missed this old PR! Have you had much of a chance to test it in the meantime? If you're pretty confident it works, I'll merge it - I don't use the sync feature myself so I'm not confident I know what to look for!

scwe commented 1 month ago

Have you had much of a chance to test it in the meantime?

Wow, what a coincidence - we actually just decided internally to switch to the fork which contains this commit. So I haven't tested it much except for in the example repo.

How would you feel about leaving this open and giving us ~1 month to hammer on it a little internally and then I'll ping you assuming all goes well?

I think that'll give me more confidence that everything is working as expected.

jph00 commented 1 month ago

Sure - take your time :)

Message ID: @.***>

scwe commented 4 days ago

@jph00 I think I'm comfortable with saying this is working well. We've been using it as part of CI for a few weeks and I've also retested to make sure I didn't break flat structured projects in the process.

jph00 commented 3 days ago

Many thanks for the update!