Cleanup source selection in fetch_nwb by defaulting to Nwbfile

[Copilot]

Summary

Streamlines the fetch_nwb source selection logic by defaulting to Nwbfile when no explicit AnalysisNwbfile foreign key exists, eliminating the need for boilerplate foreign key declarations in many table definitions.

Problem

The current _nwb_table_tuple method uses a clunky three-step check:

1. Check for _nwb_table attribute
2. Check for -> AnalysisNwbfile in table definition
3. Check for -> Nwbfile in table definition
4. Raise NotImplementedError if none found

This requires explicit foreign key declarations even when Nwbfile (the most common case) is needed.

Solution

Simplified the logic to assume data comes from Nwbfile unless AnalysisNwbfile foreign key exists:

Before:

resolved = getattr(self, "_nwb_table", None) or (
    AnalysisNwbfile if "-> AnalysisNwbfile" in self.definition
    else Nwbfile if "-> Nwbfile" in self.definition 
    else None  # Would raise NotImplementedError
)

After:

resolved = getattr(self, "_nwb_table", None) or (
    AnalysisNwbfile if "-> AnalysisNwbfile" in self.definition
    else Nwbfile  # Sensible default
)

Key Changes

• Simplified logic: Default to Nwbfile instead of failing when no explicit foreign key found
• Maintained precedence: _nwb_table attribute still takes highest priority
• Updated documentation: Clarified the new behavior and precedence order
• Comprehensive testing: Added test coverage for all scenarios including the new default behavior

Backward Compatibility

✅ Fully backward compatible - all existing table patterns continue to work:

• Tables with _nwb_table = Nwbfile (unchanged)
• Tables with -> AnalysisNwbfile FK (unchanged)
• Tables with explicit -> Nwbfile FK (unchanged)
• Tables with no FK reference (now default to Nwbfile instead of failing)

Benefits

• Less boilerplate: No need to add -> Nwbfile to table definitions in common cases
• More predictable: Sensible default behavior instead of cryptic errors
• Easier to use: New tables automatically work with fetch_nwb without extra configuration

Fixes #1292.

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[samuelbray32]

spyglass imports in tests need to be from fixtures. This should be imported from from the common test fixture

[Copilot]

Fixed in 81437c6. Changed to use the common fixture instead of direct import.

[samuelbray32]

Similar to above. Use the utils and common fixtures for these imports

[Copilot]

Fixed in 81437c6. Updated to use SpyglassMixin and common fixtures instead of direct imports.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.65%. Comparing base (4a198e5) to head (289b1cd).
⚠️ Report is 14 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1348      +/-   ##
==========================================
- Coverage   69.66%   69.65%   -0.01%     
==========================================
  Files         103      103              
  Lines       12532    12531       -1     
==========================================
- Hits         8730     8729       -1     
  Misses       3802     3802              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[CBroz1]

@samuelbray32 So, this changes the default case, yeah? For a table that (a) as no fk-ref and (b) has no explicitly set _nwb_table attr, calling fetch_nwb will now fetch an upstream Nwbfile.

• What happens for a params table with no upstream fk-ref to Nwbfile?
• Are there cases where fetching the experimental file upstream doesn't make sense? DLCModel comes to mind as a place where maybe we can link to the nwbs for the training videos, but it's misleading to suggest we're fetching the model's nwb when we haven't built the infrastructure to use the nwb extension

Sorry I'm late to the game on this - could've thought through more carefully when the issue was posted

[samuelbray32]

@CBroz1 That's a good point. Especially with things like parameter tables it could lead to unfortunate things like opening all raw nwb files in the database. It might be better to close this PR for now and revisit