Repository navigation
feat: Copy over the code from diffpy.srfit.structure to here - #1
Open
cadenmyers13 wants to merge 13 commits into
Open
cadenmyers13 wants to merge 13 commits into
cadenmyers13 wants to merge 13 commits into
Conversation
Copy src/diffpy/srfit/structure/ and its tests (test_diffpyparset, test_objcrystparset, test_sgconstraints, plus LaMnO3.stru) verbatim from diffpy.srfit upstream/main (8d8931a). No edits, so imports still point at diffpy.srfit.structure and are fixed in the next commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace diffpy.srfit.structure imports and references with diffpy.cmistructure in the copied modules and tests, and make the package importable and testable: - __init__.py: restore the package header and __version__ import. - src/diffpy/__init__.py: add extend_path so diffpy.cmistructure does not shadow the rest of the diffpy namespace. - pyproject.toml: point the entry point at cmistructure_app:main. - requirements: declare diffpy.srfit, diffpy.structure, diffpy.srreal, and pyobjcryst for tests. - tests: add datafile and availability fixtures and tests/__init__.py; remove the template functions.py and its test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
diffpy.cmistructure has never been released, so it has no users to migrate. Drop constrainAsSpaceGroup, the three StretchModeParameter setConst forwarders in objcrystparset, their deprecation messages and diffpy.utils._deprecator imports, and the test for the constrainAsSpaceGroup warning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rename every camelCase function and method in the package to snake_case (e.g. canAdapt -> can_adapt, getScatterers -> get_scatterers, restrainBondLength -> restrain_bond_length, struToParameterSet -> stru_to_parameter_set), along with their call sites and tests. No deprecation aliases are kept since the package has not been released. Rewrite the public docstrings to the NumPy style: imperative one-line summary, typed Parameters/Returns/Raises sections with descriptions starting with "The", and the nonstandard "Managed Parameters" and "Inherited Attributes" sections folded into Attributes. Also replace ObjCrystMoleculeParSet.get_lattice's calls to newPar, which no longer exists on srfit's ParameterSet, with new_parameter. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace abbreviations in the public API with full words, e.g. stru_to_parameter_set -> structure_to_parameter_set, stru -> structure, scat -> scatterer, res -> restraint, par -> parameter, cryst -> crystal, idx -> index, sg -> space_group, matoms -> mutated_atoms, angunits -> angle_units, sgpars -> space_group_parameters, and latpars/xyzpars/adppars -> lattice/xyz/adp_parameters. Parameters that have default values (sig, w, sgoffset, constrainlat, adpsymbols, isosymbol, ...) and the default container name "sgpars" are unchanged so keyword-argument callers keep working. Class and module names are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pyobjcryst and diffpy.structure are declared test and runtime dependencies, so the tests should fail rather than silently skip when they are missing. Remove the pyobjcryst_available and diffpy_structure_available fixtures, their skip checks, and the cctbx import guard in the already-disabled xtestCreateSpaceGroup. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rewrite the assertions in test_diffpyparset, test_objcrystparset and test_sgconstraints as `assert actual_* == expected_*`, with the code under test on the actual side, and add C#/Expected comments for each logical stage. Related checks (e.g. which coordinates are fixed, which atoms moved) are compared as one dict or list so a failure reports every mismatch at once. What each test checks, including tolerances, is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Merge the diffpy.cmistructure header into the original diffpy.srfit header of each module and test moved from diffpy.srfit.structure. The Columbia University copyright and original author are kept, as the BSD license requires, and the header points to both LICENSE.rst and LICENSE_DANSE.rst. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The modules moved from diffpy.srfit.structure are covered by the DANSE license from Columbia University, which must be distributed with them. Copy it verbatim from diffpy.srfit; MANIFEST.in already includes LICENSE*.rst in the sdist. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Author
|
@sbillinge ready for review. Please check that the headers in the source and tests files have the copyright correctly formated. |
Replace calls that diffpy.structure has deprecated for removal in 4.0.0 with their snake_case equivalents: GetSpaceGroup -> get_space_group, positionFormulas -> position_formulas, UFormulas -> u_formulas, IsSpaceGroupIdentifier -> is_space_group_identifier and, in the tests, Lattice.setLatPar -> set_latt_parms. All of these exist in diffpy.structure 3.4.0 and later. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Author
|
Some problems with testing... investigating |
Contributor
|
is it that we haven't published to CF one of the dependencies with the deprecated variable names? |
Author
|
@sbillinge Yeah, something along those lines. In concept this migration is straightforward but in practice I need to think about it carefully. Im working with claude to come up with a good plan |
Contributor
|
we don't need tests passing in principle to get this started. So don't stress oo much. As long as we don't break what is in produciton. |
Author
|
@sbillinge Yeah you're right. I'm working on some srfit cleanup today to ensure we're good over there before proceeding here which I think is necessary to some extent. I think we are going to need a small release of srfit before much work can be done here. Otherwise, we'll make additions here, fixes in srfit, then will have to make changes here again which adds extra unnecessary work |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I took some liberty to
actual_* == expected_*with case namesAlso had to revert the max python version to 3.13 because of srreal