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

ci: update lints Ruff & docformatter #3130

Merged
merged 16 commits into from
May 7, 2024
Merged

ci: update lints Ruff & docformatter #3130

merged 16 commits into from
May 7, 2024

Conversation

Borda
Copy link
Contributor

@Borda Borda commented Feb 20, 2024 •
edited
Loading

Issue #, if available:
dofformatter gave a false positive signal; it has lots of issues, but as --check was missing, it just fixed without any real code change and so passed green even if it failed, see for example https://github.com/awslabs/gluonts/actions/runs/7975893111/job/21775154024?pr=3130

Description of changes:

validation of the actual Ruff & Docfroatter on the latest codebase

Please tag this pr with at least one of these labels to make our release process faster:
this is just fixing the actual linting issue, but the better and ext step shall be #3111

cc: @jaheba @kashif @lostella

@Borda Borda force-pushed the fix/lints branch 2 times, most recently from b6fe800 to 848c646 Compare February 20, 2024 16:08
@Borda Borda marked this pull request as ready for review February 20, 2024 16:34
@Borda Borda mentioned this pull request Feb 20, 2024
Open
@Borda
Copy link
Contributor Author

Borda commented Feb 20, 2024

I do think that the stile check is related, but anyway would be good to update Black (sending PR #3131) so we better see if it is related 🐿️

@Borda
Copy link
Contributor Author

Borda commented Feb 20, 2024

not sure what Black does not like:

would reformat /home/runner/work/gluonts/gluonts/src/gluonts/maybe.py
would reformat /home/runner/work/gluonts/gluonts/src/gluonts/mx/block/snmlp.py
would reformat /home/runner/work/gluonts/gluonts/src/gluonts/nursery/tsbench/src/tsbench/analysis/utils/misc.py
would reformat /home/runner/work/gluonts/gluonts/src/gluonts/torch/util.py

@Borda Borda changed the title ci: update lints ci: update lints Ruff & docformatter Feb 20, 2024
@kashif
Copy link
Contributor

kashif commented Feb 26, 2024

@Borda unrelated, can you kindly send me an email kashif.rasul@gmail.com I wanted to ask you something about lightning?

@Borda
Copy link
Contributor Author

Borda commented Apr 5, 2024 •
edited
Loading

@kashif @lostella resolved conflicts, could you pls check it 🦩

it would also need to update the required checks, remove flake8 and add lint (ruff) instead

lostella
lostella reviewed Apr 8, 2024
View reviewed changes
Copy link
Contributor

@lostella lostella left a comment

Choose a reason for hiding this comment

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

Thanks @Borda! Some comments are inline

src/gluonts/evaluation/metrics.py Outdated Show resolved Hide resolved
src/gluonts/mx/distribution/distribution.py Show resolved Hide resolved
src/gluonts/mx/distribution/inflated_beta.py
Comment on lines +99 to +100
r"""Zero Inflated Beta distribution as in Raydonal Ospina, Silvia L.P.
Ferrari: Inflated Beta Distributions.
Copy link
Contributor

Choose a reason for hiding this comment

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

Some docstrings are changed to have a newline after the quote symbols, but this change does the opposite: is this because it's a raw string here?

src/gluonts/nursery/robust-mts-attack/pts/modules/gaussian_diffusion.py Outdated Show resolved Hide resolved
src/gluonts/nursery/tsbench/src/tsbench/evaluations/tracking/job.py Outdated Show resolved Hide resolved
src/gluonts/nursery/tsbench/src/tsbench/recommender/utils/pareto.py Outdated Show resolved Hide resolved
src/gluonts/torch/distributions/binned_uniforms.py Outdated Show resolved Hide resolved
@Borda
Copy link
Contributor Author

Borda commented Apr 12, 2024 •
edited
Loading

@lostella, thank you for your detailed read and for pointing out all the issues... I will try to address them
just to clarify, the docformatter even was running was ignored, so this is just trying to re-enable it properly :)

I think that Ruff's rule D could help here

@Borda Borda requested a review from lostella April 15, 2024 16:54
@Borda
Copy link
Contributor Author

Borda commented Apr 18, 2024 •
edited
Loading

@lostella, do you have any suggestions on what we should do with the remaining cases? 🦩

@lostella
Copy link
Contributor

@Borda just one comment then this can go in 🚀

@Borda @lostella
format
f59c491 
Co-authored-by: Lorenzo Stella <lorenzostella@gmail.com>
@Borda
Copy link
Contributor Author

Borda commented Apr 19, 2024

@lostella committed your suggestion, thx :)

@Borda
Copy link
Contributor Author

Borda commented Apr 25, 2024 •
edited
Loading

@lostella hope all is fine now, and with this resolved, I could finish the other PR 🦩

could you pls replace flake8 with Ruff & Docformat in the required checks?

@Borda
Copy link
Contributor Author

Borda commented May 7, 2024

@lostella, how are you doing? mind have look :)

lostella
lostella approved these changes May 7, 2024
View reviewed changes
Copy link
Contributor

@lostella lostella left a comment

Choose a reason for hiding this comment

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

Looks good! 🚀 Thanks @Borda (especially for the patience 🙂)

@lostella lostella merged commit b63fc05 into awslabs:dev May 7, 2024
20 checks passed
@Borda Borda deleted the fix/lints branch May 7, 2024 12:16
kashif pushed a commit to kashif/gluon-ts that referenced this pull request Jun 15, 2024
@Borda @lostella @kashif
ci: update lints Ruff & docformatter (awslabs#3130)
76a6540 
*Issue #, if available:*
`dofformatter` gave a false positive signal; it has lots of issues, but
as `--check` was missing, it just fixed without any real code change and
so passed green even if it failed, see for example
https://github.com/awslabs/gluonts/actions/runs/7975893111/job/21775154024?pr=3130

*Description of changes:*

validation of the actual Ruff & Docfroatter on the latest codebase


**Please tag this pr with at least one of these labels to make our
release process faster:**
this is just fixing the actual linting issue, but the better and ext
step shall be awslabs#3111

cc: @jaheba @kashif @lostella

---------

Co-authored-by: Lorenzo Stella <lorenzostella@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lostella lostella lostella approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@Borda @kashif @lostella