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

Removed blue formatter from docs an .toml files #198

Closed
wants to merge 6 commits into from

Conversation

Vishalk91-4
Copy link

Pull Request description

Removed Blue formater from docs, auto-commint, pre-commit and various .toml files

Fixes #186

How to test these changes

  • Run in cli

Pull Request checklists

This PR is a:

  • bug-fix
  • new feature
  • maintenance

About this PR:

  • it includes tests.
  • the tests are executed on CI.
  • the tests generate log file(s) (path).
  • pre-commit hooks were executed locally.
  • this PR requires a project documentation update.

Author's checklist:

  • I have reviewed the changes and it contains no misspelling.
  • The code is well commented, especially in the parts that contain more complexity.
  • New and old tests passed locally.

Additional information

Reviewer's checklist

Copy and paste this template for your review's note:

## Reviewer's Checklist

- [ ] I managed to reproduce the problem locally from the `main` branch
- [ ] I managed to test the new changes locally
- [ ] I confirm that the issues mentioned were fixed/resolved

@Vishalk91-4
Copy link
Author

@Saransh-cpp , could you please review this PR

Copy link
Member

@Saransh-cpp Saransh-cpp left a comment

Choose a reason for hiding this comment

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

Thanks for working on this, @Vishalk91-4! See my comments below -

src/scicookie/hooks/post_gen_project.py Outdated Show resolved Hide resolved
src/scicookie/profiles/base.yaml Outdated Show resolved Hide resolved
src/scicookie/profiles/base.yaml Outdated Show resolved Hide resolved
src/scicookie/profiles/base.yaml Outdated Show resolved Hide resolved
src/scicookie/profiles/base.yaml Outdated Show resolved Hide resolved
src/scicookie/profiles/base.yaml Outdated Show resolved Hide resolved
src/scicookie/profiles/base.yaml Outdated Show resolved Hide resolved
src/scicookie/profiles/base.yaml Outdated Show resolved Hide resolved
src/scicookie/profiles/base.yaml Outdated Show resolved Hide resolved
tests/smoke/auto-format-tools.sh Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
README.md Outdated
@@ -20,7 +20,7 @@ for a Python package.
- Licenses supported: MIT, BSD 3 Clause, ISC License, Apache Software License 2.0, and GPL 3
- Documentation engines: mkdocs, sphinx, jupyter-boook
- Test library: pytest, hypothesis
- Auto format code tool: blue and black
- Auto format code tool: ruff
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we support ruff as an auto formatter tool?

@Vishalk91-4
Copy link
Author

@Saransh-cpp, I have applied the mentioned changes, could you check them now

@Saransh-cpp
Copy link
Member

I don't think you pushed your changes?

@Vishalk91-4
Copy link
Author

I don't think you pushed your changes?

I'm sorry, actually I was fixing it between my classes, so missed it

src/scicookie/profiles/base.yaml Outdated Show resolved Hide resolved
@@ -258,14 +257,6 @@ def http2ssh(url):
return url.replace("/", ":", 1)


def validation():
Copy link
Member

Choose a reason for hiding this comment

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

Please check if this function is being called anywhere and remove all those occurences

src/scicookie/profiles/base.yaml Outdated Show resolved Hide resolved
src/scicookie/profiles/base.yaml Outdated Show resolved Hide resolved
src/scicookie/profiles/base.yaml Outdated Show resolved Hide resolved
src/scicookie/profiles/base.yaml Outdated Show resolved Hide resolved
src/scicookie/profiles/base.yaml Outdated Show resolved Hide resolved
src/scicookie/profiles/base.yaml Outdated Show resolved Hide resolved
src/scicookie/profiles/base.yaml Outdated Show resolved Hide resolved
Copy link
Member

@Saransh-cpp Saransh-cpp left a comment

Choose a reason for hiding this comment

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

Thanks, @Vishalk91-4! You can look at the failing tests locally or by clicking the details button on GitHub to see what should improved in this PR before it is ready to merge.

Please let us know if you are stuck at a test and we can try debugging that locally!

@@ -1,5 +1,3 @@
{%- if cookiecutter.use_blue -%}
{%- set QUOTE = "'" -%}
{%- elif cookiecutter.use_black -%}
Copy link
Member

Choose a reason for hiding this comment

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

For example, this should be if now

@Vishalk91-4
Copy link
Author

Makim file: .makim.yaml
/home/vishalk91-4/.local/lib/python3.10/site-packages/_pytest/config/__init__.py:331: PluggyTeardownRaisedWarning: A plugin raised an exception during an old-style hookwrapper teardown.
Plugin: helpconfig, Hook: pytest_cmdline_parse
UsageError: usage: pytest [options] [file_or_dir] [file_or_dir] [...]
pytest: error: unrecognized arguments: --template
  inifile: /mnt/c/Users/Vishal - IIT BHU/Documents/GitHub/scicookie/pyproject.toml
  rootdir: /mnt/c/Users/Vishal - IIT BHU/Documents/GitHub/scicookie
For more information see https://pluggy.readthedocs.io/en/stable/api_reference.html#pluggy.PluggyTeardownRaisedWarning
  config = pluginmanager.hook.pytest_cmdline_parse(
ERROR: usage: pytest [options] [file_or_dir] [file_or_dir] [...]
pytest: error: unrecognized arguments: --template
  inifile: /mnt/c/Users/Vishal - IIT BHU/Documents/GitHub/scicookie/pyproject.toml
  rootdir: /mnt/c/Users/Vishal - IIT BHU/Documents/GitHub/scicookie

Traceback (most recent call last):
  File "/tmp/tmpe2nw53_k.makim", line 1, in <module>
    pytest --template -s -vv src/scicookie tests
 

  RAN: /home/vishalk91-4/.local/bin/xonsh /tmp/tmpe2nw53_k.makim

  STDOUT:


  STDERR:

@Saransh-cpp, facing this error while running tests, tried deleting template, faced another error where it couldn't find cookiecutter

@Saransh-cpp
Copy link
Member

These would be issues with your local setup. It would be tough for me to reproduce them locally. Are you able to use scicookie locally to create packages?

You can always see the error logs using GitHub Actions, for instance the failing auto-format-tools.sh test is here - https://github.com/osl-incubator/scicookie/actions/runs/7720857565/job/21046425608?pr=198

@Naman-Priyadarshi
Copy link
Contributor

Hi @Vishalk91-4, are you currently working on this?

@Vishalk91-4
Copy link
Author

Hi @Vishalk91-4, are you currently working on this?

Not particularly, you can work on it now

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.

Remove support for blue
3 participants