Algorithms are not supposed to change flag (decision from a long time ago), unless they are meant to change them (of course). The noted inconsistency should be fixed, but then it should be fixed that atom and bond flags are also not changed.

So, I would suggest to not apply this patch, but:

1. add a test to make sure atom and bond flags are not changed either
2. (likely) fix this class that it does not change atom and bond flags

@johnmay, do you agree?

What was the issue with the current code, i.e. which flags were being set/cleared?

A lot of the QSAR descriptors are in a bit of a mess as to avoid the non-modify rule a lot of the implementations simply clone the input (expensive).

I consider aromaticity model has to be applied only in caller code because unexpected aromaticity model change can affect the other descriptor results using the same molecule.
In this case, removing Aromaticity.cdkLegacy().apply(molecule); is the simple way but breaks existing codes.

Although I don't like it, the simple fix here is to clone the container (see others).

IMO the better design choice is to decide on a "normal form" of a molecule, it has: a) bond orders assigned, b) non-null hydrogen counts, c) ring flags assigned (isInRing), and d) some aromaticity applied, e) maybe atom types assigned but I don't think they're needed - everything else should be internal to the descriptor (e.g. using a thread local cache). Although there are different aromaticity models and some particular descriptor may need some specific model - providing the caller can choose and assign a different model thats okay.

I agree with John. I consider it is by cdk's design.
Anyway, molecule's flags are not required to keep in this method under the design.

This is fixed in #407.