DEP Remove pytorch from environment.yml
State: closed

- [ ] closes #49796 (Replace xxxx with the GitHub issue number)
- [ ] [Tests added and passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#writing-tests) if fixing a bug or adding a new feature
- [ ] All [code checks passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit).
- [ ] Added [type annotations](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#type-hints) to new arguments/methods/functions.
- [ ] Added an entry in the latest `doc/source/whatsnew/vX.X.X.rst` file if fixing a bug or adding a new feature.


Labels:
name: Dependencies
description: Required and optional dependencies

Comments:
@mroeschke looks like you'd added `pytorch` in: https://github.com/pandas-dev/pandas/pull/47287/files

Would you be OK with this change?
Thanks for taking a look

As far as I understand, a lock file would speed up the solving of the environment, but wouldn't affect its size

What I'm trying to tackle here is the size of the environment, which I think is bigger than that of practically any other open source project

If you're -1 on commenting and uncomment the pytorch dependency, would you be OK with having a separate environment for for asv checks (which would contain pytorch) and removing pytorch from environment.yml?
I've tried this again, making a separate environment for the `asv` check

Like this:
- the environment for ASV checks can be a lot smaller;
- it avoids the comment / uncomment hack;
- the `environment.yml` file which contributors use could be 2GB lighter, and this makes a real difference if you don't have a fast internet connection

> Unless there's an urgent need to remove this

Not urgent, but the PyDataGlobal sprints are coming soon (early December), and I'd be pretty keen to remove 2GB of dependencies which are irrelevant to 99.9% of contributions (probably not an exaggeration) by then
> Thanks for taking a look
> 
> As far as I understand, a lock file would speed up the solving of the environment, but wouldn't affect its size
> 
> What I'm trying to tackle here is the size of the environment, which I think is bigger than that of practically any other open source project
> 
> If you're -1 on commenting and uncomment the pytorch dependency, would you be OK with having a separate environment for for asv checks (which would contain pytorch) and removing pytorch from environment.yml?

If python=same is the problem, can we just remove that part of the asv command, so that asv can create its own environment? Otherwise, we wouldn't have coverage for that in the CI(I've definitely forgot to update Cython before in the asv.conf.json and CI didn't fail).
Yeah that's a good point - I tried it https://github.com/pandas-dev/pandas/actions/runs/3512450800/jobs/5884163088 but got:

```
ram [7110656]: · Unknown branch main in configuration
Error: Process completed with exit code 1.
```
with no further info. Any ideas? Looking further into this, debugging here https://github.com/pandas-dev/pandas/pull/49814

> If you're -1 on commenting and uncomment the pytorch dependency, would you be OK with having a separate environment for for asv checks (which would contain pytorch) and removing pytorch from environment.yml?

I would generally be OK with splitting up `environment.yml` for different "work topics" in pandas like:

* code-checks
* docs
* web
* benchmarks
* tests-required
* tests-optional
* tests-downstream

then an environment can be assembled based on install 1 or more of the dependency files depending on what is worked on.

(Right, conda-lock doesn't necessarily simplify the env size, but it greatly reduces the solve speed which can also be a limiting factor)
> If python=same is the problem, can we just remove that part of the asv command, so that asv can create its own environment? Otherwise, we wouldn't have coverage for that in the CI(I've definitely forgot to update Cython before in the asv.conf.json and CI didn't fail).

🥳 🎉 finally got it, just needed to remove the `default` channel (which is consistent with the `environment.yml` file, which only has the `conda-forge` channel), and add a minimum pin for matplotlib, else it would fail to resolve. But at least now there's coverage for asv creating its own environment in CI. I also had to add `git fetch origin main:main`

So, now, we have:
- lighter `environment.yml` file
- asv checks let `asv` create its own environment, so that's covered in CI too
- none of the initially-suggested hacks

@mroeschke would you be OK with this solution? (if not, no worries, hopefully there's something else we could work towards)
I think this is OK. That's a pretty big dependency to have, and doesn't seem like it is even tested in our CI?
I'd be okay removing pytorch (potentially for now) by:

1. Still commenting it out of the `environment.yml` file (@jreback 's suggestion) hopefully moving toward more targeted environment files. 
2. Just removing the pytorch usage in the ASV benchmark and test in `test_downstream.py` all together

My main (stickler of a) motivation is that when I did a lot of dependency cleanup a while back for the CI, local dependencies in `environment.yml` were misaligned with the CI dependency files. And in theory, one should be able to have an environment that runs everything the CI does (test & docs & checks) since its the ultimate gatekeeper
Not sure I follow, wouldn't

https://github.com/pandas-dev/pandas/blob/0168e27843efb96b56202f22fbe5dd720c346368/asv_bench/asv.conf.json

count as a more targeted environment file? Because then `asv` would create its own environment, and so starting with what's in `environment.yml` you'd still be able to run everything

I've gone with your suggestion anyway
> count more targeted environment file?

Not sure if I completely follow now. That file only sets up the environment for ASV IIUC. If `pytorch` was removed from `environment.yml`, `test_downstream.py:test_torch_frame_construction` (now removed) wouldn't run
> > count more targeted environment file?
> 
> Not sure if I completely follow now. That file only sets up the environment for ASV IIUC. If `pytorch` was removed from `environment.yml`, `test_downstream.py:test_torch_frame_construction` (now removed) wouldn't run

my bad, I was only thinking about the ASV
thanks for your review / approval! merging then