[ricardonpa]

Hi @amkrajewski,

I believe I have addressed everything we need to release v0.5.0.

There is still an issue with passing the tests on the Mac machine, but we ran out of credits before I could check if it was resolved. If we change the privacy of the repository to public, I might be able to complete this asap. Then I could also set up Read the Docs and Codecov.

Let me know your thoughts.

Best!

[ricardonpa]

@amkrajewski ping!

[amkrajewski]

@ricardonpa I went through all changes line by line. In general, most things look great and I left notes and nitpicks. One major thing we need to adjust are the tests which regressed heavily in terms of integration testing, especially test_core - I left detailed comments but if you used LLM for consolidating them please note it missed a lot of improtant stuff. Other than that, mostly minor stuff and some questions.

[ricardonpa]

Hi @amkrajewski, I think I just need ur input in the "POSCAR homogenization" comment. All the rest should be okay to proceed!

[Copilot]

Pull request overview

Copilot reviewed 39 out of 44 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (4)

crystall/core.py:364

• _aggregate_pipeline expects msgs to be a sequence of full status strings, but here a plain string is passed. In Python, ('matching native candidates') is just a string (iterable of characters), so the spinner will cycle one character at a time. Pass a 1-tuple or list instead.
  crystall/core.py:587
• Same issue as above: msgs=('matching native candidates') is a string, not a tuple of strings, so yaspin text will iterate character-by-character. Use a 1-tuple or list.
  crystall/core.py:500
• The final summary print in run_MPDD(..., target_prototypes='non-native') uses len(aggregated_data) after looping over collections, but aggregated_data only contains the last collection’s facet output. This makes the reported “Total number of unique prototypes” misleading (and can be flat-out wrong when multiple collections are used).
  crystall/core.py:2262
• This module-level “Package Metadata” block overwrites Python’s reserved module attributes like __name__ and __doc__. Reassigning __name__ is particularly risky (it affects repr/introspection and can break pickling/import assumptions). Also, importlib.resources.files("crystall").joinpath("pyproject.toml") currently always fails because there is no crystall/pyproject.toml in the package (confirmed by repo contents), so the fallback values are always used.

[amkrajewski]

🥳