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.

Sign up for GitHub

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

Jump to bottom

Modernize ruff #342

Merged
merged 2 commits into from
Nov 28, 2024
Merged

Modernize ruff #342

merged 2 commits into from
Nov 28, 2024

Conversation

taldcroft
Copy link
Member

@taldcroft taldcroft commented Nov 2, 2024 •
edited by jeanconn
Loading

Description

  • Apply latest ruff and use ruff.toml and ruff-base.toml.
  • Spend some time minimizing the exclusions and fixing a few real issues.

Requires

  • ska_sun, ska_helpers @ master

Interface impacts

None

Testing

Unit tests

  • Mac
(ska3) ➜  kadi git:(ruff-modern) ✗ git rev-parse HEAD 
154c92eccb1d12d58e95b2995927cc55f5187802
(ska3) ➜  kadi git:(ruff-modern) env PYTHONPATH=/Users/aldcroft/git/ska_sun:/Users/aldcroft/git/ska_helpers pytest              
============================================= test session starts ==============================================
platform darwin -- Python 3.11.8, pytest-7.4.4, pluggy-1.4.0
rootdir: /Users/aldcroft/git
configfile: pytest.ini
plugins: cov-5.0.0, timeout-2.2.0, anyio-4.3.0
collected 182 items                                                                                            

kadi/commands/tests/test_commands.py ................................................................... [ 36%]
...............                                                                                          [ 45%]
kadi/commands/tests/test_states.py .......................x.........................                     [ 71%]
kadi/commands/tests/test_validate.py ...................                                                 [ 82%]
kadi/tests/test_events.py ..........                                                                     [ 87%]
kadi/tests/test_occweb.py ......................                                                         [100%]

================================== 181 passed, 1 xfailed in 68.22s (0:01:08) ===================================

Independent check of unit tests by Jean

  • Linux
(ska3-masters) jeanconn-fido> git rev-parse HEAD
154c92eccb1d12d58e95b2995927cc55f5187802
(ska3-masters) jeanconn-fido> python -c "import ska_sun; print(ska_sun.__version__);"
3.15.0
(ska3-masters) jeanconn-fido> python -c "import ska_helpers; print(ska_helpers.__version__);"
0.17.0
(ska3-masters) jeanconn-fido> pytest
======================================================================== test session starts =========================================================================
platform linux -- Python 3.11.8, pytest-7.4.4, pluggy-1.4.0
rootdir: /proj/sot/ska/jeanproj/git
configfile: pytest.ini
plugins: timeout-2.2.0, anyio-4.3.0
collected 182 items                                                                                                                                                  

kadi/commands/tests/test_commands.py ..................................................................................                                        [ 45%]
kadi/commands/tests/test_states.py .......................x.........................                                                                           [ 71%]
kadi/commands/tests/test_validate.py ...................                                                                                                       [ 82%]
kadi/tests/test_events.py ..........                                                                                                                           [ 87%]
kadi/tests/test_occweb.py ......................                                                                                                               [100%]

============================================================= 181 passed, 1 xfailed in 199.59s (0:03:19)

Functional tests

No functional testing.

taldcroft
taldcroft commented Nov 2, 2024
View reviewed changes
kadi/occweb.py
@@ -362,7 +364,9 @@ def get_occweb_dir(path, timeout=30, cache=False, user=None, password=None):
astropy Table
Table of directory entries
"""
html = get_occweb_page(path, timeout=timeout, cache=cache)
html = get_occweb_page(
path, timeout=timeout, cache=cache, user=user, password=password
Copy link
Member Author

Choose a reason for hiding this comment

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

This was a real bug, the user and password kwargs were being ignored.

kadi/events/json_field.py
@@ -28,7 +28,7 @@ def default(self, obj):
if isinstance(obj, Decimal):
return str(obj)
elif isinstance(obj, datetime.datetime):
assert settings.TIME_ZONE == "UTC"
assert settings.TIME_ZONE == "UTC" # noqa: S101
Copy link
Member Author

Choose a reason for hiding this comment

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

This code was just copied from somewhere else so leave it be.

kadi/commands/validate.py
@@ -733,7 +733,7 @@ def get_overlap_mask(times: np.ndarray, intervals: Table):


@functools.lru_cache(maxsize=1)
def get_command_sheet_exclude_intervals(state_key: str = None) -> Table:
def get_command_sheet_exclude_intervals() -> Table:
Copy link
Member Author

Choose a reason for hiding this comment

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

The state_key kwarg must be a leftover since the function does not use it and the call to this function does not provide it.

jeanconn
jeanconn reviewed Nov 21, 2024
View reviewed changes
@taldcroft taldcroft merged commit 7a350cd into master Nov 28, 2024
4 checks passed
@taldcroft taldcroft deleted the ruff-modern branch November 28, 2024 00:34
@javierggt javierggt mentioned this pull request Dec 9, 2024
Merged
@taldcroft taldcroft mentioned this pull request Jan 2, 2025
Merged
2 tasks
@javierggt javierggt mentioned this pull request Jan 23, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeanconn jeanconn jeanconn approved these changes

@javierggt javierggt Awaiting requested review from javierggt

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@taldcroft @jeanconn