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

[MAINTENANCE] Add extras to setup.cfg for optional deps #17

Merged
merged 9 commits into from
Jan 14, 2025

Conversation

tyler-hoffman
Copy link
Contributor

@tyler-hoffman tyler-hoffman commented Jan 14, 2025

What this does

  • Adds optional deps as extras in setup.cfg
  • Uses that pattern instead of the previous test requirements.txt file
  • Opts CI jobs into just the deps they need.
  • Moves around a couple imports that were previously at the top level and started to fail for jobs that didn't have optional deps installed.

Note on the mypy deps
Our type definitions for dataframes allow for both pandas and spark. The base gx install includes pandas, and the spark extras also need to be included for mypy to work.

What this doesn't do
CORE-730 says we should install the deps and perform a minimal proof that things work. I think the maintenance burden for a reasonable proof is sufficiently high to make this not worth the effort at the current time. We do have proof that the approach works, as the following extras are used by CI: tests, lint, postgresql, spark.

@tyler-hoffman tyler-hoffman force-pushed the m/CORE-730/optional-deps branch from 4cc27fb to 03302aa Compare January 14, 2025 14:57
@tyler-hoffman tyler-hoffman force-pushed the m/CORE-730/optional-deps branch from 03302aa to b92c91d Compare January 14, 2025 15:45
@tyler-hoffman tyler-hoffman force-pushed the m/CORE-730/optional-deps branch 2 times, most recently from 5bf0cee to 8b68e7c Compare January 14, 2025 17:08
@tyler-hoffman tyler-hoffman force-pushed the m/CORE-730/optional-deps branch from 8b68e7c to c139d75 Compare January 14, 2025 17:12
@tyler-hoffman tyler-hoffman changed the title Add extras to setup.cfg [MAINTENANCE] Add extras to setup.cfg for optional deps Jan 14, 2025
ruff==0.8.3
pytest==8.3.4
pytest-mock==3.14.0
great-expectations[spark, spark-connect]>=1.3.1

Choose a reason for hiding this comment

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

Do we really need spark in lint?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. We do because spark dataframes are in the type signature of at least one of the operators. I don't see a way around it, but LMK if you do.

Choose a reason for hiding this comment

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

Ok...that leads me to ask the question: do we not support pandas for this operator?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do support pandas. And that is part of the base gx install, so it's already included. I'll update the description on this PR, since these are good things to be clear about.

@tyler-hoffman tyler-hoffman merged commit 5773c0c into main Jan 14, 2025
9 checks passed
@tyler-hoffman tyler-hoffman deleted the m/CORE-730/optional-deps branch January 14, 2025 21:32
@joshua-stauffer
Copy link
Member

this looks great, thanks @tyler-hoffman .

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.

3 participants