[FEATURE] Use more granular requirements-dev-xxx.txt files #4327
Conversation
|
✔️ Deploy Preview for niobium-lead-7998 ready! 🔨 Explore the source changes: 3b141eb 🔍 Inspect the deploy log: https://app.netlify.com/sites/niobium-lead-7998/deploys/62310c1cada47c0008419e4f 😎 Browse the preview: https://deploy-preview-4327--niobium-lead-7998.netlify.app |
…irements-dev-files
…irements-dev-files
setup.py
Outdated
| with open("requirements-dev-lite.txt") as f: | ||
| dev_lite = f.read().splitlines() | ||
|
|
||
| req_boto = [req for req in dev_lite if req.startswith("boto")] | ||
| req_sqlalchemy = [req for req in dev_lite if req.startswith("sqlalchemy")] | ||
|
|
||
| with open("requirements-dev-airflow.txt") as f: | ||
| req_airflow = f.read().splitlines() | ||
|
|
||
| with open("requirements-dev-arrow.txt") as f: | ||
| req_arrow = f.read().splitlines() | ||
|
|
||
| with open("requirements-dev-athena.txt") as f: | ||
| req_athena = f.read().splitlines() |
There was a problem hiding this comment.
Could this logic be encapsulated in a for-loop or function of sorts?
tests/test_packaging.py
Outdated
| with open(file_relative_path(__file__, "../requirements-dev-lite.txt")) as req: | ||
| requirements_dev_lite = { | ||
| f'{line.name}{"".join(line.specs[0])}' for line in rp.parse(req) | ||
| } | ||
|
|
||
| with open(file_relative_path(__file__, "../requirements-dev-contrib.txt")) as req: | ||
| requirements_dev_contrib = { | ||
| f'{line.name}{"".join(line.specs[0])}' for line in rp.parse(req) | ||
| } |
There was a problem hiding this comment.
Same note here - the granularity we're adding adds a lot of verbosity and I'd love to simply things into a loop/function/dict if possible.
There was a problem hiding this comment.
Not changing this file
There was a problem hiding this comment.
Any reason why? I'm okay leaving it be but this seems like a good candidate for a refactor.
There was a problem hiding this comment.
I ended up refactoring it...
requirements.txt
Outdated
| pandas>=0.23.0 | ||
| pyparsing>=2.4,<3 | ||
| python-dateutil>=2.8.1 | ||
| pytz>=2015.6 # <---- should we update to something in the recent few years? |
There was a problem hiding this comment.
Can we resolve this before merging?
There was a problem hiding this comment.
Comment was added so it could be discussed in PR
requirements-dev-lite.txt
Outdated
| boto3>=1.9 # <------ should this be 1.14 (see extras_require["s3"] in setup.py) | ||
| # <------ should this be 1.17.106 (see constraints-dev.txt) |
There was a problem hiding this comment.
Please resolve these comments before merging.
There was a problem hiding this comment.
Comment was added so it could be discussed in PR
requirements-dev-airflow.txt
Outdated
| @@ -0,0 +1 @@ | |||
| apache-airflow[s3]>=1.9.0 | |||
There was a problem hiding this comment.
I think we should get rid of the airflow file... I only added it because there was an extras_require entry in setup.py, but it's not documented and nobody would miss it.
There was a problem hiding this comment.
Sounds good to me but maybe reach out to @talagluck first just to confirm!
setup.py
Outdated
| lite = results.pop("lite") | ||
| results["boto"] = [req for req in lite if req.startswith("boto")] | ||
| results["sqlalchemy"] = [req for req in lite if req.startswith("sqlalchemy")] | ||
| results["airflow"] += results["boto"] |
There was a problem hiding this comment.
This line would be deleted too
…hub.com/great-expectations/great_expectations into feature/GREAT-500/add-notebooks-for-rbp * 'feature/GREAT-500/add-notebooks-for-rbp' of https://github.com/great-expectations/great_expectations: [FEATURE] Use more granular requirements-dev-xxx.txt files (#4327) standardise on include and exclude columns list for profiler (#4424) [MAINTENANCE] Continue chipping away at warnings (#4422) [MAINTENANCE] Add docstrings and type hints to `Anonymizer` (#4419) Replace "sampling_method" with "estimator", since the latter is a more fitting name for the parameter. (#4420) [BUGFIX] Adding `--spark` flag back to `azure-pipelines.yml` compatibility_matrix stage. (#4418) [MAINTENANCE] Refactor out unnecessary Anonymizer child classes (#4408) [BUGFIX] Pin `pytest-azurepipelines` due to breaking changes made in latest release
Changes proposed in this pull request:
Definition of Done
Please delete options that are not relevant.