Skip to content

chore: deprecate scatterFactoringTableByType method#176

Open
stevenhua0320 wants to merge 2 commits intodiffpy:mainfrom
stevenhua0320:deprecate-scatterFactoringTableByType
Open

chore: deprecate scatterFactoringTableByType method#176
stevenhua0320 wants to merge 2 commits intodiffpy:mainfrom
stevenhua0320:deprecate-scatterFactoringTableByType

Conversation

@stevenhua0320
Copy link
Copy Markdown
Contributor

@stevenhua0320 stevenhua0320 commented Apr 9, 2026

@sbillinge We need to keep this open for a while as right now we didn't list diffpy.srreal as a dependency. When we release the new version of diffpy.srreal we can add this back to our requirements and rerun the test. This closes #137

@sbillinge
Copy link
Copy Markdown
Contributor

I think we should move the lookup tables out of diffpy.srreal. if we do that, can we remove srreal as a dependency? This would be much better.

@stevenhua0320
Copy link
Copy Markdown
Contributor Author

I think somewhere it also uses diffpy.srreal method to do PDFCalculator and BVSRestraint. I'm not sure whether we should not include this as the dependency? If we don't need it then we might also keep these functionalities unused. Moreover, I'm afraid some of the issues in our current issues might arise from the use of diffpy.srreal.

@sbillinge
Copy link
Copy Markdown
Contributor

I think the next step here is to make an inventory of everywhere srreal is used in srfit and make a plan for removing it.

In the meantime there is not much to do tbecause we can't push out 3.14 for srreal until we migrate from boost and if we can't push out srfit until we update srreal we also can't push out a 3.14 version of diffpy.cmi.

@cadenmyers13 please review this. My suggestion is that put all this activity on hold for the time being, keep ourselves pinned at 3.13 for diffpy in general (some subpackages are already 3.14 but will also build with 3.13 so that is ok). Then we create a new operation-release-everything (ore) that we tackle all this with new students that I will be recruiting shortly. It would be great if the current team could mentor the new team, but then wind down responsibilities to focus on their own projects. This has been a very successful period to get our codes into a more robust and good place that we can build on. We can still work on different projects of interest to update different packages and fix bugs etc., but the global push to 3.14 everything can wait for the new ore. How does that sound?

We may want to update user facing docs in 1diffpy.cmi to mention that it is only working up to 3.13 before we step away.

@cadenmyers13
Copy link
Copy Markdown
Contributor

@stevenhua0320

diffpy.srreal imports are found in files under diffpy.srfit.pdf and diffpy.srfit.structure (see screenshot). All these imports are found within functions and are not at the top of the file, so diffpy.srreal isnt listed as a dependency of srfit.
Screenshot 2026-04-14 at 11 33 29 AM

I agree with @sbillinge about holding off. Also, there might be a way to dynamically display the versions of Python which a package supports in the docs. I think Bob did it in scikit-package docs maybe.

@sbillinge
Copy link
Copy Markdown
Contributor

Let's look into pulling these out. It doesn't really matter where the import statements are

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.69%. Comparing base (43d83a1) to head (1405760).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #176       +/-   ##
===========================================
+ Coverage   71.63%   85.69%   +14.06%     
===========================================
  Files          25       25               
  Lines        3437     3439        +2     
===========================================
+ Hits         2462     2947      +485     
+ Misses        975      492      -483     
Files with missing lines Coverage Δ
tests/test_pdf.py 96.23% <100.00%> (+48.41%) ⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore: replace the call to setScatteringFactorTableByType

3 participants