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

Cleanup xrdutil #663

Merged
merged 16 commits into from
Jul 3, 2024
Merged

Cleanup xrdutil #663

merged 16 commits into from
Jul 3, 2024

Conversation

ZackAttack614
Copy link
Collaborator

Clean up the xrdutil.py file.

  • Add formatting with black.
  • Add type annotations.
  • Remove USE_NUMBA checks.
  • Remove unnecessary functions and variables.
  • Add @deprecated decorator to warn users of functions marked for sunset.

@ZackAttack614 ZackAttack614 requested a review from psavery July 1, 2024 20:52
@pep8speaks
Copy link

pep8speaks commented Jul 1, 2024

Hello @ZackAttack614! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-07-02 18:15:31 UTC

self.save_eta_ome_maps(self, filename)

@staticmethod
def save_eta_ome_maps(eta_ome, filename):
def save_eta_ome_maps(eta_ome, filename: str) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing type annotation on eta_ome

hexrd/deprecation.py Outdated Show resolved Hide resolved
Comment on lines 1166 to 1167
lmin = [processWavelength(e) for e in minEnergy]
lmax = [processWavelength(e) for e in maxEnergy]
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like minEnergy and maxEnergy got swapped in the change here. Can you change them back?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolved

Copy link

codecov bot commented Jul 2, 2024

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

Copy link
Collaborator

@psavery psavery left a comment

Choose a reason for hiding this comment

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

LGTM.

@ZackAttack614 ZackAttack614 merged commit 8119695 into master Jul 3, 2024
8 checks passed
@ZackAttack614 ZackAttack614 deleted the cleanup-xrdutil branch July 3, 2024 15:12
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.

4 participants