Skip to content

feat: Add Class to fuse connections#203

Merged
filimarc merged 23 commits intomainfrom
feature/fuse_connectivity
Jan 9, 2026
Merged

feat: Add Class to fuse connections#203
filimarc merged 23 commits intomainfrom
feature/fuse_connectivity

Conversation

@filimarc
Copy link
Contributor

@filimarc filimarc commented Dec 1, 2025

Describe the work done

It is implemented a PostConnectivityHook that helps the user to merge chains of connectivity set.
The core class is FuseConnectivity that implement the logic for merging two connection tabels and the method create_connectivity_set that builds the connectivity tree of a list of connection provided and creates a new connectivity set for every root-leaf pair.

Two strategies are proposed:

  • The user can directly provide the list of connectivity set to merge
  • The user specify which cell to bypass

List which issues this resolves:

closes #183

Tasks

  • Added tests
  • Updated documentation

📚 Documentation preview 📚: https://bsb--203.org.readthedocs.build/en/203/


📚 Documentation preview 📚: https://bsb-core--203.org.readthedocs.build/en/203/

@github-actions github-actions bot added the feat label Dec 1, 2025
@codecov
Copy link

codecov bot commented Dec 1, 2025

Codecov Report

❌ Patch coverage is 95.52239% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.51%. Comparing base (3133628) to head (b575b6d).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
packages/bsb-core/bsb/postprocessing.py 95.45% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #203      +/-   ##
==========================================
+ Coverage   78.46%   85.51%   +7.05%     
==========================================
  Files         196       86     -110     
  Lines       17953    11015    -6938     
  Branches     2100     1308     -792     
==========================================
- Hits        14087     9420    -4667     
+ Misses       3313     1303    -2010     
+ Partials      553      292     -261     

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@filimarc filimarc requested a review from Helveg December 15, 2025 12:52
Helveg
Helveg previously requested changes Dec 15, 2025
Copy link
Contributor

@Helveg Helveg left a comment

Choose a reason for hiding this comment

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

I don't like using class maps for generic strategy attributes, I mostly use them when the components that are being used represent some easily understood object or concept like a "layer", "cone", ... here your classmap is just the snake case of your class name, then to me that doesn't warrant using a classmap.

The documentation is good! Maybe cover even more cases and edge cases? The same ones that I've mentioned in the docstring.

If possible, I think all this information should go into the docstring? That way all the information is in 1 place. I'm not sure I am in favor of maintaining separate lists of component explanations anymore (we did that in BSB2&3 and it was always very poorly maintained and incomplete). If we make the docstring a complete documentation then the information about the class isn't spread out in multiple places.

@filimarc
Copy link
Contributor Author

I will improve the docstrings and and add some examples in the documentation. But i think that to move all the documentation in the docstrings will require a lot of refactoring (for instance the code / config snippets).
If we want to tackle this, then it should be done later in a separate PR.

@Helveg
Copy link
Contributor

Helveg commented Dec 16, 2025

I agree that including such extensive documentation into the docstring might require a bit too much rethinking of what you wrote, even though docstrings fully support RST iirc.

This PR places a new link here (red rectangle), while I would rather place it as another domain (red stripe):

image

Like the other domains then the concept of post processing can be introduced, and your 2 explanations can be moved to their own "List of postprocessing hooks" the way the other domains do it. At least that way it's analog to what we have elsewhere and we can table the rest of the discussion for another time 😄

@drodarie do you agree we should add the postprocessing docs analogously the placement and connectivity domain docs?

@drodarie
Copy link
Contributor

@Helveg, Yes we agree on the location of the postprocessing section and how to split the content.
Also, the refactor that you suggested on the docstrings, while being feasible as the documentation is in rst format, entails a lot of work. We have to find the time for this 😅

@Helveg
Copy link
Contributor

Helveg commented Dec 17, 2025

Yes let's table that part for now!

What I think important for the quality of the documentation for now is that:

  • We have the postprocessing domain
  • The domain contains a first article that explains the general concept of postprocessing
  • The article continues in a trend that explains more common and simple use cases in sections, and the use cases advance as the article continues
  • The domain contains additional guides that explain large concepts in the domain (the way that the Placement domain has a Placement set article) that also start simple and then delve deeper (not sure that Postprocessing even has that, it's a small domain)
  • The "list of postprocessing hooks" is a separate article

Oh, also, explain how postprocessing deals with parallellization?

@filimarc filimarc requested a review from Helveg December 18, 2025 12:45
@filimarc filimarc dismissed Helveg’s stale review January 7, 2026 12:33

He had already reviewed

@filimarc filimarc requested a review from Helveg January 7, 2026 12:34
Copy link
Contributor

@Helveg Helveg left a comment

Choose a reason for hiding this comment

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

Looks greeeaaaat!!! Here are some typos. I would recommend to pass all new documentation text through an LLM.

@filimarc filimarc requested a review from Helveg January 8, 2026 11:27
@filimarc
Copy link
Contributor Author

filimarc commented Jan 8, 2026

I made grammar check with Gemini, do not know why bsb-yaml doc is failing now...

@Helveg
Copy link
Contributor

Helveg commented Jan 8, 2026

The docs failed randomly, I'm rebuilding them to double check. @drodarie this one is ready to merge :)

@filimarc filimarc merged commit 715b59e into main Jan 9, 2026
27 checks passed
@Helveg Helveg deleted the feature/fuse_connectivity branch January 15, 2026 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create a postconnectivity hook for merging connectivity sets

3 participants