Move StreamLines to analysis and remove MDAnalysis.visualization

[hmacdope]

Potentialy move streamlines to an MDAKit and out of core or remove it. EDIT:

• [ ] move MDAnalysis.visualization.streamlines to MDAnalysis.analysis.streamlines
• [ ] deprecate MDAnalysis.visualization
• [ ] #4545

[orbeckst]

@tylerjereddy what are your views on your streamlines?

[tylerjereddy]

The functionality is associated with a publication (https://doi.org/10.1039/C3FD00145H) that has been cited ~50 times, and includes a mention of the MDAnalysis code and corresponding VMD plugin that support the paper. So I suppose it may be nice to keep it alive somehow.

That said, I wouldn't be surprised if it wasn't heavily used in practice.

I probably have pretty different views than most MDA core devs on the tradeoffs associated with splitting functionality off into sub-libs and having less eyes on it for maintenance. I guess I don't see a major benefit to splitting it off, we've only had pretty minor patches to it over the years, most/all of them from me? I suppose if it were the only remaining component using matplotlib there might a decent case for it, but I'd still not be super keen on migrating it.

FWIW, I don't even like having tests in a separate package, but the ships have sailed.

[hmacdope]

Tagging @orbeckst here.

[orbeckst]

I wouldn’t want to completely remove streamlines without replacement. But with the broader push towards a leaner 3.0 it makes sense to move it into a kit. I see it a bit like PSA: it’s published and in principle we have the expertise but it’s fairly specialized code that’s not used a lot. With 3.0 we have a good opportunity to make such changes.

Opinions from the other MDAKits team members @lilyminium @fiona-naughton @IAlibay ?

[tylerjereddy]

I think it is fine to proceed with your grant initiative, consider me outvoted, I'm just pointing out that there's really no substantial benefit from the standpoint of the streamlines functionality so I'm -0.5 on principle, and I'm probably a bit less likely to maintain something once it drops out of core.

We're also less likely to notice if something does go wrong--after all, I believe that's one of the main purposes of extracting "stuff" from core, to reduce the burden of all the different analysis things causing breaks to CI/releases/builds, and I don't think you can do that with no tradeoffs, there's usually at least some cost.

Those occasional breaks to "core" actually tell us when we may need to maintain something. If you just have i.e., occasional crons for things that aren't used much anyway, the motivation isn't really there anymore, it isn't breaking core when it breaks, and if nobody complains for a year or two then it may well just stay broken/rot. I suspect you've already thought of that and you probably understand what I mean, it is just way easier to ignore cron/integration notifications that aren't breaking core for some tiny analysis lib, etc.

[IAlibay]

Mmm I don't know on my end - the point of the mdakits is that it's meant to be optional / on the authors to make decisions. We've been a bit poor at doing this so maybe this is no longer the aim :/

[lilyminium]

@tylerjereddy I agree with you that there's a lot of downsides to splitting out code from the main library, and the practical outcome in this case seems to be generally negative (less maintenance for streamlines, and unless we also port the dihedral analyses out like Ramachandran we'll still have matplotlib as a dependency). That being said, conceptually streamlines feels like an MDAKit and doesn't feel like it fits into the "lean 3.0" vision. So unless we decide not to commit to "lean 3.0" I'm still in favour of not keeping streamlines as it currently is in visualisation.

An alternative could be to refactor streamlines into something that fits into the analysis module, and then keep that in there?

[orbeckst]

An alternative could be to refactor streamlines into something that fits into the analysis module, and then keep that in there?

I like the suggestion to move it to MDAnalysis.analysis.streamlines.

[orbeckst]

At the last EOSS4 meeting we decided that we would like to move streamlines into analysis and discontinue a separate visualization module as this seems to be out of scope for the main library. However, streamlines itself can be maintained inside analysis — the effort to maintain it there is lower than to maintain it as a separate MDAKit. matplotlib is not a heavy dependency and could be made optional (#4336).

Does this sound reasonable to everyone?

[IAlibay]

I would be keen to get @tylerjereddy's blessing on this decision.

matplotlib is not a heavy dependency and could be made optional

May I correct this to "will be made optional"?

[orbeckst]

May I correct this to "will be made optional"?

sure

[tylerjereddy]

Sure that sounds fine. To be clear I'm not going to have a fit if you just rip it out entirely either. But yeah, this gives it a chance of remaining usable a bit longer. Honestly I'm not sure I've ever used it since Matthieu asked me to code it, but the same is true of most other stuff I guess, I pretty much exclusively do u = MDAnalysis.Universe and then everything ends up custom after that.

[lilyminium]

Thanks @tylerjereddy! I'll start refactoring this unless anyone else would be very keen to do so.

[orbeckst]

@lilyminium is #4549 blocking the move? Or can we move it and wait for #4549 to get fixed?

[lilyminium]

I had interpreted "move StreamLines to analysis" as also refactoring streamlines to be AnalysisBased. It could certainly be moved without refactoring, but if refactoring it's somewhat more work to keep the bug than fix it. I could probably just fix #4549 as part of this.