What are our immediate priorities for this fork?

[connortann]

The SHAP Community Fork is really taking off, which is terrific! We're quite close to getting the test suite to pass, which is a really great milestone. This will make it a fair bit easier for us to review & merge in new PRs.

Once we get the CI passing, what do you think should be our next priority? My current thinking is to focus on fixing the majority of warnings & deprecation messages; however, interested to hear any other views. In particular, perhaps we should consider making a first release sooner rather than later.

[dsgibbons]

Hi @connortann, once we get that blue tick on master, I'll send an email to slundberg to see if he is open to merging our fork and expanding the maintainer pool over there. If we don't have luck with merging back into the main repository, we can start looking into a PyPI release.

Yes, I think deprecations and warnings are a good start (the Boston dataset issue needs addressing pretty soon so that the README can be brought up-to-date). There still seem to be some tests skipping in GitHub Actions, so it would be nice to get those to work (I think we can configure GPU-enabled runners too? github/roadmap#505). I also wonder if we can speed up GitHub Actions using dependency caching? https://github.com/actions/setup-python#caching-packages-dependencies - I don't know much about how this works though.

[dsgibbons]

[thatlittleboy]

Before the release (assuming slundberg doesn't reply favourably), I suppose we should also update the LICENSE and copyright notices on the repo / docs / pypi project metadata. See, e.g. https://gist.github.com/fbaierl/1d740a7925a6e0e608824eb27a429370.

[connortann]

Some other ideas of things we should do before publishing to pypi:

• Add some preliminary installation instructions (i.e. pip install from git)
• Define a versioning system for the fork
• Add some version numbers & git tags (such as RC or beta releases)

[dsgibbons]

I haven't heard back from Scott yet. I'll send him a follow up email later this week. If we don't hear anything by the end of next week, then I propose that we start investigating versioning/release.

[dsgibbons]

@connortann @thatlittleboy could you send me an email? I have things to discuss with you both. See my profile for my email address.

[thatlittleboy]

For me, my mind is still on 1) porting PRs -- not everything, obviously, but I still have a couple of PRs that I want to knock out before our first official release; and 2) linting and autoformatting, then 3) one round of dogfood-ing & read-through of the entire docs to clean up any glaring mistakes. In that order.

I'm open to beta releases in the meantime, however, if you guys want to test the release & publish workflow.

[connortann]

Sounds good. On point 2) - are you sure autoformatting is wise? On the thread on #21 , we discussed how adopting autoformatting could be a major barrier in reconciling PRs between forks, as the vast majority would encounter merge conflicts.

One big benefit of autoformatting is getting minimal diffs between versions. Given the vast majority of open PRs are on the parent repo, adopting autoformatting would have the opposite effect. So IMO it would be quite detrimental for our fork in practial terms, despite the aesthetic beauty of black-formatted code.

I so agree that fixing the the major pyflakes errors (as reported via ruff) as a great aim for linting though.

[thatlittleboy]

On point 2) - are you sure autoformatting is wise? On the thread on #21 , we discussed how adopting autoformatting could be a major barrier in reconciling PRs between forks, as the vast majority would encounter merge conflicts.

So I guess I misunderstood our conclusion in #21. For the purposes of porting PRs, I only imagined it to last up till our first release (i.e., the existing PRs on slundberg's repo, those that are feasible / non-controversial to port over have been ported over).

My expectation is that there will be fewer and fewer PRs over at slundberg/shap over time, and so the number of PRs that we need to "port" over will also be decaying over time. Our situation is not like neovim v.s. vim, or yt-dlp v.s. youtube-dl, where there needs to be continuous porting of PRs from the main repo to the forked repo. If slundberg/shap became active again, there's no real reason for this fork to exist; we're just spreading resources thin.

adopting autoformatting would have the opposite effect. So IMO it would be quite detrimental for our fork in practial terms,

The main issue with introducing black is not with diffs, but with conflicts when porting PRs, IMO. Diffs (for PRs within this fork) will be smaller, not larger if we adopt black. Because there's less disagreement on code styles across contributors.
It would've been painful if we adopted black at the beginning of our journey, because of the high intensity of PR porting (hence why I disagreed with introducing black in #21 ). But I don't think the frequency of porting will be that high after this.

+ I think if we have proper code formatting (compared to slundberg/shap) in our code base, it would actually encourage contributors to contribute over here instead.

[connortann]

Yep, so to be clear: diffs on PRs within the fork will be slightly smaller; but diffs between forks will be much much larger.

I suppose the question is whether it's valuable to retain the ability to compare between forks longer-term, even after a first release. Adopting black makes this really pretty difficult.

Not only would it impede porting the remaining 98 open PRs from upstream, but it makes it hard for slundberg in future to identify what has changed. From his perspective, understanding the diff will be paramount when reviewing and accepting changes from this fork.

Are the marginal productivity gains that we'd get from adopting autoformatting worth these downsides?

[thatlittleboy]

I suppose the question is whether it's valuable to retain the ability to compare between forks longer-term, even after a first release.

Agreed. My view is that it's not valuable. I'm willing to put aside black for the first release if you're concerned about the portability of PRs -- and I get this concern, I really do. But we can't be tied down by an inactive project forever.

Not only would it impede porting the remaining 98 open PRs from upstream

I don't think we will port over all open PRs from upstream, only the feasible / non-controversial ones. Which is what I plan to do before the first release.

it makes it hard for slundberg in future to identify what has changed. From his perspective, understanding the diff will be paramount when reviewing and accepting changes from this fork.

True, so let's see what's the outcome from @dsgibbons ' interaction with slundberg first. If slundberg doesn't show interest in maintaining his repo any further, would you then be open to introducing black here? Since the argument of porting the changes here back to slundberg's repo would no longer hold true.

Are the marginal productivity gains that we'd get from adopting autoformatting worth these downsides?

Depends on the severity of the downsides. And we clearly disagree on that :)

ps. I think it's funny how black is meant to prevent disagreements about style and here we are arguing at length about whether to introduce it or not.

[connortann]

That sounds like a good way forward 👍

Haha yes indeed. Well, we're saved from having to discuss a style guide, so that's a win. I think the discussion is more about managing merge conflicts than about style.

[dsgibbons]

This fork was a community-led effort to fix issues in the popular SHAP repository in 2022/23. At the time, the main repository had stalled. The project owner was the sole maintainer, and PRs were blocked by a failing test suite. In June 2023, the main collaborators on this fork (@thatlittleboy, @connortann, and @dsgibbons) were given write-access on the main repository. This fork now just exists as a place for me to work on my own SHAP contributions. Thank you all for your efforts in bringing SHAP back to life!