Skip to content

Conversation

@dagewa
Copy link
Member

@dagewa dagewa commented May 16, 2025

Also update deprecation message to announce that the latter will be completely removed later.

Update deprecation message to announce that the latter will be
completely removed later.
@dagewa dagewa requested a review from ndevenish May 16, 2025 15:38
@dagewa
Copy link
Member Author

dagewa commented May 16, 2025

Seems sensible to stage this. So, this changes the default, then we can remove all pathlib=True in dials_data calls across DIALS and dxtbx. Finally we can remove the choice entirely.

@ndevenish
Copy link
Member

I'd probably go full nuclear; outright remove; catch and raise an error for it's presence in kwargs, just always return Path.

Do a major version bump (which I think by the dials-data deployment we do here, e.g. update pyproject.toml) so that it's a semantic version break.

@dagewa
Copy link
Member Author

dagewa commented May 16, 2025

Ok, fair enough - will go for that

@dagewa
Copy link
Member Author

dagewa commented Oct 27, 2025

@ndevenish I rediscovered this. Are you happy with it, now it has full removal and version bump?

@codecov
Copy link

codecov bot commented Nov 4, 2025

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 49.69%. Comparing base (60ceb6e) to head (3cbee3c).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #509      +/-   ##
==========================================
- Coverage   56.25%   49.69%   -6.56%     
==========================================
  Files           6        6              
  Lines         368      330      -38     
==========================================
- Hits          207      164      -43     
- Misses        161      166       +5     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ndevenish
Copy link
Member

Yes, good legacy cruft to remove

@dagewa dagewa merged commit e2b2a92 into main Nov 4, 2025
21 checks passed
@dagewa dagewa deleted the pathlib.Path-default branch November 4, 2025 20:10
@ndevenish
Copy link
Member

I'm going to revert this as this has killed all of dials (well, 737 tests). Having a redundant parameter should at best be a warning, not a ValueError.

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.

3 participants