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

Fix typo in docs #1300

Merged
merged 1 commit into from
Aug 11, 2023
Merged

Conversation

nathanjmcdougall
Copy link
Contributor

At the moment, I suspect the CI isn't passing, since even changes which are purely cosmetic are failing on the ray modin tests.

This is a single character typo fix in source code which should hopefully trigger the CI and prove it is failing, so that it can be fixed.

Signed-off-by: Nathan McDougall <nmcdougall@tonkintaylor.co.nz>
@cosmicBboy
Copy link
Collaborator

thanks @nathanjmcdougall yeah there seems to be an issue with modin-ray with the lateset version of ray... needa look into it

@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Patch coverage has no change and project coverage change: -16.56% ⚠️

Comparison is base (57d8269) 93.72% compared to head (52a08d1) 77.16%.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1300       +/-   ##
===========================================
- Coverage   93.72%   77.16%   -16.56%     
===========================================
  Files          90       90               
  Lines        6697     6697               
===========================================
- Hits         6277     5168     -1109     
- Misses        420     1529     +1109     
Files Changed Coverage Δ
pandera/backends/pandas/builtin_checks.py 95.52% <ø> (ø)

... and 33 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cosmicBboy
Copy link
Collaborator

Currently in CI hell: #1303

Need to figure out how to handle environment dependencies that gracefully handles all the different versions of python, pandas , etc.

@nathanjmcdougall
Copy link
Contributor Author

nathanjmcdougall commented Aug 11, 2023

Need to figure out how to handle environment dependencies that gracefully handles all the different versions of python, pandas , etc.

This is a bit out of my depth potentially, but here are some thoughts:

It's helpful to separately provide the abstract dependencies (currently in environment.yml and setup.py) from the concrete/pinned dependencies (currently not provided). As it stands, the CI might fail, not because of any changes to the code, but simply because a new version of this-or-that package has been released in the background.

I think the ideal would be that the environments are frozen (including all transitive dependencies) for all development and CI. Every dependency would have an exact version number specified, rather than a range. This would ensure everyone & everything is using the same environment (for each Python version), regardless of whether new versions of dependencies have been released. Then dependabot can be used to detect when the frozen/pinned version is out-of-date, at which point we get a separate PR to update the version, which may break the CI but at least the breakage would be confined only to that one PR rather than having collateral damage across all PRs.

An (ideal?) approach

Ideally we don't duplicate the Python package dependencies in both setup.py and elsewhere.
We can create new extras_require groups to include the dev dependencies which are currently listed as paragraphs in environment.yml.

For example:
_extras_require = {
    "dev-pkg" : [
        "twine"
    ],
    "dev-check": [
        "black >= 22.1.0",
        "isort >= 5.7.0",
        "pylint <= 2.17.3",
        "pre_commit",
    ],
    "dev-typecheck": [
        "mypy <= 0.982",
        "types-click",
        "types-pyyaml",
        "types-pkg_resources",
        "types-requests",
        "types-pytz",
    ],
    "dev-test": [
        "pytest",
        "pytest-cov",
        "pytest-xdist",
        "pytest-asyncio",
        "pytz",
        "xdoctest",
        "nox",
        "importlib_metadata", # required if python < 3.8
    ],
    "dev-fastapi-test": [
        "uvicorn",
        "python-multipart",
    ],
    "dev-perf-test": [
        "asv >= 0.5.1",
    ],
    "dev-doc": [
        "sphinx",
        "sphinx-panels",
        "sphinx-autodoc-typehints <= 1.14.1",
        "sphinx-copybutton",
        "recommonmark",
        "jupyterlite_sphinx",
        "furo",
    ],
    "dev-notebook": [
        "jupyterlite",
    ]
    ... # other extra_requires groups for the end-user
}

These setup.py sections would then serve as the abstract dependencies.

We would then want to create separate files for different combinations of:

  • Different Python versions
  • Different versions of major packages (i.e. pandas)
  • Different package managers (conda/mamba vs pip)
  • Maybe different OSes, but this seems overkill.

To do this, (for each version combination) we could use pip-tools' pip-compile to generate requirements.txt file from setup.py.
Then maybe create a script in the opposite direction to generate_pip_deps_from_conda.py which creates a conda environment.yml from the requirements.txt, perhaps like so.
Note that this would mean that you would only use conda for installing Python itself. pip would handle all package installation. Otherwise pip-tools would get confused.

Suggested Approach

For now, I suggest keeping the parallelism between setup.py and environment.yml/requirements.txt. However, I think we should create a new requirements.in file which contains basically the same contents as environment.yml, and then start using pip-tools to generate concrete/pinned requirements.txt files, and then some other script to generate conda environment.yml files from that. We should be able to ad-hoc over-ride the pandas version as needed.
There would be one for each version of Python + pandas combination, i.e. requirements-3.7-pd1.3.0.txt, requirements-3.8-pd2.0.1.txt, etc. and corresponding environment-3.7-pd1.3.0.yml, environment-3.8-pd2.0.1.yml, etc.

@cosmicBboy
Copy link
Collaborator

Great recommendation! Yes, I think pip-compile would be the way to go here... the only thing to account for is extras like geopandas, which has OS-specific dependencies which can just use conda/mamba in CI to install for simplicity.

I just disabled modin-ray CI tests here #1303 just to unblock the PRs

@cosmicBboy
Copy link
Collaborator

Created a ticket: #1306

@cosmicBboy cosmicBboy changed the title Pass the CI (currently failing?) Fix typo in docs Aug 11, 2023
@cosmicBboy cosmicBboy marked this pull request as ready for review August 11, 2023 21:34
@cosmicBboy cosmicBboy merged commit 2330c45 into unionai-oss:main Aug 11, 2023
11 of 40 checks passed
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.

2 participants