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

Formula parser & alias cleanup & rich-based help #27

Merged
merged 39 commits into from
Apr 5, 2022
Merged

Conversation

o-smirnov
Copy link
Member

Matched set with caracal-pipeline/scabha2#21.

Copy link
Collaborator

@SpheMakh SpheMakh left a comment

Choose a reason for hiding this comment

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

I like the new features, especially xrun_asyncio!

Let's remove (comment out) the scabha dependency from setup.py for now.

@@ -9,13 +9,19 @@
requirements = ["pyyaml",
"nose>=1.3.7",
"future-fstrings",
"scabha @ git+https://github.com/caracal-pipeline/scabha2",
"scabha >= 0.7.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Then we should just remove the scabha dependency for now. Because scabha 0.7.0 is not on pypi, and it makes no sense to require it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Or we could just merge the scabha PR and make a release?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That works for me

from stimela.config import ConfigExceptionTypes
import click
import logging
import os.path, yaml, sys
import os.path, yaml, sys, asyncio
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not kosher. Unless importing from the same module/library, each import should be on a new line

Copy link
Member Author

Choose a reason for hiding this comment

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

PEP-8 says "Imports should usually be on separate lines", so I tend to interpret that loosely. Particularly when a large number of the usual standard library suspects are imported, like import os, os.path, sys etc. I just find the import section gets too big.

@o-smirnov
Copy link
Member Author

@SpheMakh please approve and merge.

@SpheMakh SpheMakh merged commit 487399a into master Apr 5, 2022
@SpheMakh SpheMakh deleted the parsers branch April 5, 2022 09:43
@o-smirnov o-smirnov linked an issue Apr 5, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants