adding cell property

[cvazz]

No description provided.

[cvazz]

This does not seem to be that easy. I fixed issues relating to pytest. Some issues remain with mypy.

I paste the relevant issues below.

meteor/iterative.py:258: error: Argument "cell" to "_iteratively_denoise_sf_amplitudes" of "IterativeTvDenoiser" has incompatible type "UnitCell | None"; expected "Sequence[float] | ndarray[tuple[Any, ...], dtype[Any]] | UnitCell"  [arg-type]
test/unit/test_iterative.py:49: error: Argument "cell" to "_tv_denoise_complex_difference_sf" of "IterativeTvDenoiser" has incompatible type "UnitCell | None"; expected "Sequence[float] | ndarray[tuple[Any, ...], dtype[Any]] | UnitCell"  [arg-type]
test/unit/test_iterative.py:67: error: Argument "cell" to "_iteratively_denoise_sf_amplitudes" of "IterativeTvDenoiser" has incompatible type "UnitCell | None"; expected "Sequence[float] | ndarray[tuple[Any, ...], dtype[Any]] | UnitCell"  [arg-type]
Installing missing stub packages:
/opt/hostedtoolcache/Python/3.11.14/x64/bin/python -m pip install lxml-stubs pandas-stubs types-Pygments types-colorama types-decorator types-defusedxml types-jsonschema types-networkx types-pexpect

My mypy version also takes issue with

meteor/utils.py:135: error: Variable "meteor.utils.SpacegroupType" is not valid as a type  [valid-type]
meteor/utils.py:135: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
meteor/utils.py:136: error: Variable "meteor.utils.CellType" is not valid as a type  [valid-type]
meteor/utils.py:136: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases

[codecov-commenter]

Codecov Report

❌ Patch coverage is 90.32258% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.36%. Comparing base (af99dd5) to head (fccb3c8).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
meteor/rsmap.py	89.28%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #140      +/-   ##
==========================================
+ Coverage   95.25%   95.36%   +0.10%     
==========================================
  Files          16       16              
  Lines         991     1014      +23     
==========================================
+ Hits          944      967      +23     
  Misses         47       47              

Flag	Coverage Δ
unittests	95.36% <90.32%> (+0.10%)	⬆️

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.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[cvazz]

@tjlane
Trying to include typing is revealing an ambiguity I feel.

There is a test (test_compute_dhkl) which requires the unit cell to accept None.
However several functions in iterative.py expect CellType (without None), leading to the above conflict.
So a decision has to be made whether None types should be acceptable in any situation.

Furthermore, I see that CellType is being checked for in various places. This does not strike me as a good idea.
All CellType (and SpaceGroups) are received and converted to gemmi.UnitCell and gemmi.Spacegroup. So this is what functions such as ._iteratively_denoise_sf_amplitudes should check for.

[tjlane]

@cvazz two excellent points

A crystallographic map does not make much sense without a corresponding cell and spacegroup, so I think it is good to require both. If Map.cell = None is only being used in the test, we can and should change the tests.

Your point about being stricter with regards to the types of the cell and spacegroup attributes is on-point as well. We should simply restrict the type hints.

Are you up for proposing a change in the format of a PR (here)?

[cvazz]

I did some further digging and it seems that depending on the "from_XX" method, the unit cell is either implied from the source data (can an mtz file have no unit cell info?), explicitly unitcell OR None, or explicitly unit cell (see below).
So it is a bit messier than just tweaking a test, i fear.

So, one would also have to make a more unified choice about what goes in and potentially test in the from mtz and from ccp4 method; and write good error messages.

   def from_structurefactor(
        cls,
        complex_structurefactor: np.ndarray,
        *,
        index: pd.Index,
        cell: CellType | None = None,
        spacegroup: SpacegroupType | None = None,
    ) -> Map: ...
    
   def from_3d_numpy_map(
        cls, map_grid: np.ndarray, *, spacegroup: Any, cell: CellType, high_resolution_limit: float
    ) -> Map: ...
    
    def from_ccp4_map(
        cls,
        ccp4_map: gemmi.Ccp4Map,
        *,
        high_resolution_limit: float,
        amplitude_column: str = "F",
        phase_column: str = "PHI",
    ) -> Map: ...
    ```