Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(deps): update dependency gemmi to >=0.5.5, <=0.7.0 #284

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

renovate[bot]
Copy link
Contributor

@renovate renovate bot commented Nov 30, 2024

This PR contains the following updates:

Package Change Age Adoption Passing Confidence
gemmi >=0.5.5, <=0.6.7 -> >=0.5.5, <=0.7.0 age adoption passing confidence

Release Notes

project-gemmi/gemmi (gemmi)

v0.7.0: 0.7.0

Compare Source

C++14 (or later) is required to build the library, C++17 (or later) to build Python bindings.
Expect breaking changes, especially in Python bindings.
The lists below are not complete, but should cover most of the changes.

Library
  • Added unified logging of warnings/errors from various gemmi functions (class Logger)
  • replaced string Model::name with int Model::num
  • mmcif: better handling of null auth_comp_id
  • fixes for mmJSON
  • Removed deprecated functions:
    • UnitCell.fractionalization_matrix and orthogonalization_matrix – use frac.mat and orth.mat
    • count_hydrogen_sites() – use has_hydrogen() or count_atom_sites(gemmi.Selection('[H,D]')
    • Grid::resample_to() – use interpolate_grid()
  • unified API of Grid interpolation functions. They now have parameter order that can be 0 (nearest value), 1 (linear interpolation), or 3 (cubic). In C++ there are also functions such as trilinear_interpolation() to ensure no overhead.
  • to_pdb: write HET records
  • Extended selection syntax with: [metals] and [nonmetals].
  • Added function set_is_metal() intended for debatable metalloids
  • improved interoperability with MMDB (a CCP4 library)
  • MonLib: removed read_cif args
  • mtz: fixed writing BATCH records
  • hydrogen placement: fixes needed for new files with metals in CCP4 Monomer Library
  • pdb: fixed reading TLS S tensor
  • Structure metadata: expanded RefinementInfo
Python
  • Python bindings migrated from pybind11 to nanobind.
    • Much lower runtime overhead, faster build times, better error diagnostics.
    • Built-in typing stubs.
    • Only Python 3.8+.
    • Sadly, no support for Buffer Protocol. It was replaced with NumPy __array__ methods.
      For NumPy, you can also use .array properties that were available also in the previous releases.
    • No implicit conversions from list to ndarray, and from bytes to string (let me know where it causes problems)
    • gemmi.ValueSigmaAsuData.value_array has now shape (N,2)
  • Added pickling support for Structure, Model, Chain, Residue, Atom, cif.Document, cif.Block.
  • Added function interpolate_position_array (#​323).
  • Python extension module is now installed into site-packages/gemmi/ (this change should be invisible to the user)
Program
  • gemmi convert --sifts-num is now more customizable
  • gemmi sf2map: added option --check (see docs)
  • gemmi cif2mtz: add a rule to spec to convert pdbx_F_calc_with_solvent to F-model (+phase)
  • gemmi xds2mtz: handles merged files from XSCALE
  • gemmi mtz2cif and merge: recognize extension .ahkl as XDS file

Configuration

📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 Ignore: Close this PR and you won't be reminded about this update again.


  • If you want to rebase/retry this PR, check this box

This PR was generated by Mend Renovate. View the repository job log.

@renovate renovate bot added the version Version issues label Nov 30, 2024
Copy link
Contributor Author

renovate bot commented Jan 12, 2025

Edited/Blocked Notification

Renovate will not automatically rebase this PR, because it does not recognize the last commit author and assumes somebody else may have edited the PR.

You can manually request rebase by checking the rebase/retry box above.

⚠️ Warning: custom changes will be lost.

@JBGreisman
Copy link
Member

JBGreisman commented Jan 12, 2025

The above commits address the changes to the gemmi namespace in v0.7.0. There are still a few remaining issues that I am having trouble reproducing with my local copy. Will look into it in more detail later

@kmdalton
Copy link
Member

kmdalton commented Jan 13, 2025

I was able to reproduce this issue locally. The cause is that calling np.array on a gemmi.ReciprocalGrid instance now returns an array with float64 dtype whereas the test_to_reciprocal_grid_gemmi expects complex64. I think np.array(gemmi_reciprocal_grid) , returns the real component of the complex numbers. There are some small numerical differences with what I get from numpy. At any rate, specifying the dtype, expected = np.array(gemmi_reciprocal_grid, dtype='complex64') produces output consistent with our methods.

I'm not sure why this reproduced on my machine and not yours, @JBGreisman. I suppose the biggest difference is probably linux vs mac os.

@kmdalton kmdalton requested a review from JBGreisman January 13, 2025 15:12
@codecov-commenter
Copy link

codecov-commenter commented Jan 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.68%. Comparing base (01286d8) to head (a6b66d0).
Report is 12 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #284      +/-   ##
==========================================
- Coverage   88.98%   88.68%   -0.30%     
==========================================
  Files          40       40              
  Lines        2841     2855      +14     
==========================================
+ Hits         2528     2532       +4     
- Misses        313      323      +10     
Flag Coverage Δ
unittests 88.68% <100.00%> (-0.30%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@wojdyr
Copy link
Collaborator

wojdyr commented Jan 13, 2025

I was able to reproduce this issue locally. The cause is that calling np.array on a gemmi.ReciprocalGrid instance now returns an array with float64 dtype whereas the test_to_reciprocal_grid_gemmi expects complex64.

That's unexpected. Is it a gemmi.ReciprocalComplexGrid?
There is also the .array property: grid.array should return the same as np.array(grid) with less typing.

@kmdalton
Copy link
Member

I was able to reproduce this issue locally. The cause is that calling np.array on a gemmi.ReciprocalGrid instance now returns an array with float64 dtype whereas the test_to_reciprocal_grid_gemmi expects complex64.

That's unexpected. Is it a gemmi.ReciprocalComplexGrid? There is also the .array property: grid.array should return the same as np.array(grid) with less typing.

Yes, sorry for the confusion. It's a gemmi.ReciprocalComplexGrid. Calling np.array on it returns the real part as a float64 array with a ComplexWarning.

[ins] In [16]: grid
Out[16]: <gemmi.ReciprocalComplexGrid(40, 40, 100)>  
[ins] In [17]: np.array(grid).dtype
<ipython-input-17-6d72511ef3d7>:1: ComplexWarning: Casting complex values to real discards the imaginary part  np.array(grid).dtype 
Out[17]: dtype('float64')

Thanks for the suggestion, I'll modify our test to use the .array attribute. This is with numpy==2.2.1 and gemmi==0.7.0.

@JBGreisman
Copy link
Member

awesome -- thanks for sorting this out, @kmdalton, and thanks for the suggestion @wojdyr.

This behavior only crops up with numpy >2, which is what the issue was with my local version. When I bump my numpy version, I see these failed tests related to the float/complex dtype.

I think this is ready to go, but please let me know if you think there are any remaining issues related to this PR.

Copy link
Member

@JBGreisman JBGreisman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

wojdyr added a commit to project-gemmi/gemmi that referenced this pull request Jan 14, 2025
one particular problem, for copy=False, was that astype(None) was called
resulting in casting complex values to real with warning:
ComplexWarning: Casting complex values to real discards the imaginary part
as reported in rs-station/reciprocalspaceship#284
@wojdyr
Copy link
Collaborator

wojdyr commented Jan 14, 2025

Thanks for spotting the issues in gemmi. I fixed the __array__ method that's used when np.array(grid) is called and re-enabled mtz.spacegroup = None – it got disabled unintentionally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
version Version issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants