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

Update ruff to v6.8 and many typos #2548

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

gdevos010
Copy link
Contributor

@gdevos010 gdevos010 commented Sep 29, 2024

Checklist before merging this PR:

  • Mentioned all issues that this PR fixes or addresses.
  • Summarized the updates of this PR under Summary.
  • Added an entry under Unreleased in the Changelog.

Fixes #2547.

Summary

update to ruff v6.8
fixed many typos

Other Information

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@gdevos010 gdevos010 changed the title Update ruff to v6.8 Update ruff to v6.8 and many typos Sep 29, 2024
Copy link
Collaborator

@madtoinou madtoinou left a comment

Choose a reason for hiding this comment

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

Thanks @gdevos010 for this PR, looks good overall.

Can you please revert the changes to the images in the notebooks? Also, found some changes in the code that might cause some issues.

Also, we need to make sur follow the migrations instruction from 0.3.6 to 0.6.8 to avoid issues.

@@ -458,12 +458,12 @@ def ensemble(
ensembled = [
self.regression_model.predict(
n=len(prediction),
series=serie,
series=series,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please revert this one, having both the element and iterable called series does not look good.

serie = pd.Series(data=values, index=time_range_MS)
ts = TimeSeries.from_series(pd_series=serie)
series = pd.Series(data=values, index=time_range_MS)
ts = TimeSeries.from_series(pd_series=series)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's revert this one as well.

@@ -116,9 +116,9 @@ def get_feature_times_target(
Helper function called by `get_feature_times` that extracts all times within a
`target_series` that can be used to create a feature and label. More specifically,
we can create features and labels for times within `target_series` that have *both*:
1. At least `max_lag = -min(lags)` values preceeding them, since these preceeding
1. At least `max_lag = -min(lags)` values presceeding them, since these preceeding
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
1. At least `max_lag = -min(lags)` values presceeding them, since these preceeding
1. At least `max_lag = -min(lags)` values preceding them, since these preceding

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please revert the changes to the images

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please revert the changes to the images

Copy link
Collaborator

Choose a reason for hiding this comment

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

Revert changes to images

pyproject.toml Show resolved Hide resolved
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.

update ruff to v6.8
2 participants