implement equality operator for wrapped classes

[pavoljuhas]

Crystal methods return wrappers to the same C++ instances, but they don't test for identity not equality:

>>>  nacl = loadObjCrystCrystal('NaCl.cif')
>>>  nacl.GetScatterer(0) is nacl.GetScatterer(0)
False
>>>  nacl.GetScatterer(0) == nacl.GetScatterer(0)
False
>>>  a = nacl.GetScatterer(0)
>>>  a.GetScatteringPower() is a.GetScatteringPower()
False
>>>  a.GetScatteringPower() == a.GetScatteringPower()
False

TODO

• fix identity test if possible
• fix equality operation

[sbillinge]

@vincefn please can you look at this. It is an old one from Pavol. Do we want to implement this or close the issue as no long needed, or put it to a leter release? Alll are fine with me. It may also bave been implemented. I didn't check. Maybe @Tieqiong could check that.

[sbillinge]

@vincefn one more question. In the rest of diffpy we have moved over stricltly to semantic version numbering (major.minor.patch). Our automated release scripts are based on this. However, I see you switched at some point to year.something.patch or sthg like that. Because the "type pattern" of your versioning (at least as long as the third integer is there) matches semantic numbering, by chance it would actually still work with our automated release scripts. Is there a reason you switched and why you like your method? I would actually be ok with either, though would prefer it personally if we could unify it back to semantic numbering. But let's discuss.

[sbillinge]

@vincefn one more thing. We are just about ready to ask you to test the "revamped" code. It is sitting on the migration branch in the git repo. We could try and do a pypi and/or conda pre-release so you could actually test a released version, or else if you like. These pre-releases are invisible to users unless they go searching for them, so a pip install pyobjscryst would still give them 2024.2.1 if we did that.

the objective is that we didn't break anything (or fix anything tbh), just brought everything into line with the rest of diffpy to facilitate maintenance and releases. @Tieqiong has tested the example scripts in the repo and they are all running fine (he had some questions that he put in separate issues e.g., #54) but the best is if you can quickly run some of your scripts and make sure we haven't broken anything. Then we can merge migration into main and move to a new release.

TL;DR:

1. please can you test the new version which is curently sitting on the migration branch
2. please let us know if you want to test it by building from sources or if you want us to release it to PyPI as a pre-release and test a fully installed version
3. when done, please let us know and we can merge migration into main

[sbillinge]

btw, @vincefn we want to make a new release of diffpy.cmi all the way to python 3.13 and so would love to have this version of pyobjcryst in that. Thiw will, for the first time, have diffpy.cmi running on windows, as well as latest python, so it is a big deal for us.

[sbillinge]

@vincefn if you are interested, we have formalized all our workflows in something we call scikit-package (https://scikit-package.github.io/scikit-package/). I need this because I maintain our code using grad students who cycle through so I want very strict standards. But we thought it might be useful for the broader scientific software community.

[vincefn]

Hi - so far I'm looking only partially at all the commits. No issue with the general principle, I of course agree standardisation helps a lot with the maintainability.

More specifically to the above question:

• regarding identity testing, a few years back I added an int_ptr() method precisely to start addressing this - I did not go all the way and add the equality operator because I did not have the time to test it for all classes (and possible corner cases), but it should now be easy to add. Feel free to add it
• for equality that's more complicated, and it probably needs implementation upstream. One cheap (programming-wise) implementation would be to test for the equality of the XMLOutput for all important classes, but that would not be efficient, and may fail for some corner cases (e.g. ordering of atoms and components..)
• As for versioning, I prefer the year.minor.patch, because the evolution of the version is very slow, and there would never be any reason to update the major number otherwise (so many project remain for 10+years to version 0.x.y it's actually meaningless) . Using year as major is a cheap way to follow how recent the release is - it's actually semantic versioning if you agree to use year for major..
• regarding a test release, I'll try to test but have little time (17 people in my group now..). I can probably test from source but I think pushing a pre-release would be a good idea just for testing.
• will you also test the pyobjcryst-feedstock building ?
• great that diffpy-cmi would work also on windows and py313, it took me a little while to get this to work under windows (the py38 migration was the painful one). At least pyobjcryst has been ready since last year.

Also:

• did you change the way the package version is written into the published package ? I always struggled with the mechanism used for the FALLBACK_VERSION (or some similar variable), so simplification would be super welcome.

Thanks for the headsup regarding scikit-package, looks interesting, I'll show this to my team.

[sbillinge]

@vincefn thanks for the detailed response.

• We will make an rc release today so you can test from that. We will send instructions.
• We will look into the generalized identity and equality testing methods. That would actually be super useful also for our other packages if it generalizes. I will put that into the next milestone though.
• OK, let's go with year.minor.patch since it doesn't break our scripts. I would make one request, which is that we always have a patch number, even when it is zero, so no 2025.1 but yes 2025.1.0. Is that ok with you (I noticed a 2024.2 in the list of releases)?
• haha, if you have 17 people in your group, don't you have 17 testers? Just kidding.....
• yes, we have workflows taking things all the way to conda-forge so we can handle the feedstock update. CF are a bit picky and annoying with pre-releases and we didn't quite figure out how to do that. There is a way but it is a bit complicated and we didn't have the energy to take it on. We could persevere, but for now we are releasing rc's to PyPI, testing the release, then pushing out hte full release all the way to CF then testing the CF carefully. It is a bit messy if something goes wrong between PyPI and CF, but we are just kind of doing our best.....
• py3.8 was very painful for us too.... we even were running into horrible GIL issues.

Yes, we have a very simplified package version workflow now that is consistent across all the packages. It seems to work in all cases we have needed to date. But we haven't tested any kind of fallback so this could be an issue......