Skip to content

Conversation

soapy1
Copy link
Contributor

@soapy1 soapy1 commented Apr 1, 2025

Description

Refs #14684

Depends on resolving conda/conda-planning#15 and conda/conda-planning#18

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@github-project-automation github-project-automation bot moved this to 🆕 New in 🔎 Review Apr 1, 2025
@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Apr 1, 2025
Copy link

codspeed-hq bot commented Apr 1, 2025

CodSpeed Performance Report

Merging #14725 will not alter performance

Comparing soapy1:dont-allow-input-file-type-mixing (8589a79) with main (eb3b481)

Summary

✅ 21 untouched benchmarks

@soapy1 soapy1 changed the title Eensure conda does not allow for mixing input file types Ensure conda does not allow for mixing input file types Apr 2, 2025
@soapy1 soapy1 force-pushed the dont-allow-input-file-type-mixing branch from a9ea1cf to c566564 Compare April 2, 2025 00:48
@soapy1 soapy1 force-pushed the dont-allow-input-file-type-mixing branch 2 times, most recently from c4d320a to 94cbb6f Compare May 6, 2025 23:34
@jezdez jezdez marked this pull request as ready for review May 9, 2025 08:01
@jezdez jezdez requested a review from a team as a code owner May 9, 2025 08:01
@jezdez jezdez force-pushed the dont-allow-input-file-type-mixing branch from b47a4b7 to 54d0e3f Compare May 9, 2025 08:48
@jezdez
Copy link
Member

jezdez commented May 9, 2025

pre-commit.ci autofix

1 similar comment
@jezdez
Copy link
Member

jezdez commented May 9, 2025

pre-commit.ci autofix

@jezdez jezdez moved this from 🆕 New to 🏗️ In Progress in 🔎 Review May 9, 2025
@jezdez jezdez self-assigned this May 9, 2025
@jezdez
Copy link
Member

jezdez commented May 9, 2025

@soapy1 I took the liberty to fill in the TODO, so we can continue with conda/conda-planning#15. Do these changes work for you?

@soapy1
Copy link
Contributor Author

soapy1 commented May 9, 2025

Looks great @jezdez !

Following the bug reproduction steps from the original issue, I get the result:

$ conda create -n testenv-mixed --file env1.txt --file env2.txt

EnvironmentFileTypeMismatchError: Cannot mix environment file formats.

'env1.txt' is a requirements.txt format file
'env2.txt' is a unknown (AttributeError) format file

Where the input files are:

env1.txt (explicit file)

@EXPLICIT
https://repo.anaconda.com/pkgs/main/linux-64/_libgcc_mutex-0.1-main.conda
https://repo.anaconda.com/pkgs/main/linux-64/ca-certificates-2025.2.25-h06a4308_0.conda

env2.txt (regular text file)

numpy

I think this is not exactly right. env1.txt is an explicit file and env2.txt is a requiremets file. This error is caused by some issues in the plugin implementations. I'll open a seperate issue for that. I don't think it should hold up merging this.

Related issues:

@soapy1 soapy1 force-pushed the dont-allow-input-file-type-mixing branch from 23a87fc to d029190 Compare May 29, 2025 17:17
jezdez
jezdez previously approved these changes May 29, 2025
@github-project-automation github-project-automation bot moved this from 🏗️ In Progress to ✅ Approved in 🔎 Review May 29, 2025
@jezdez
Copy link
Member

jezdez commented May 29, 2025

This just needs a news file!

@soapy1 soapy1 force-pushed the dont-allow-input-file-type-mixing branch from b964dcb to 3a7bf33 Compare May 29, 2025 17:21
@soapy1
Copy link
Contributor Author

soapy1 commented May 29, 2025

Calling out that this PR does not fully fix the issue described in #14684. I can confirm that this PR together with conda/conda-planning#47 does fully resolve the issue. Using the env1.txt and env2.txt files specified in the issue:

$ conda create -n testenv-mixed --file env1.txt --file env2.txt

EnvironmentFileTypeMismatchError: Cannot mix environment file formats.

'env1.txt' is a explicit format file
'env2.txt' is a requirements.txt format file

@jezdez
Copy link
Member

jezdez commented May 30, 2025

@soapy1 Makes sense, I think we can merge this separately, and then close the ticket once they landed.

@jezdez
Copy link
Member

jezdez commented May 30, 2025

Ok, I see the concern of @soapy1 and @jjhelmus in #14820 about not merging that, so I have the sense that we'll end up stalling everything to get the environment class refactor in first. This is a traditional "perfect is the enemy of the good" moment, we need to start somewhere and most of the PRs around the environment changes are just snapshots in time until the next release anyways. Remember that we have the 25.5.x release branch in case we need to do patch release in between.

@jezdez jezdez merged commit 46e35b6 into conda:main Jun 3, 2025
75 checks passed
@github-project-automation github-project-automation bot moved this from ✅ Approved to 🏁 Done in 🔎 Review Jun 3, 2025
soapy1 added a commit to soapy1/conda that referenced this pull request Jun 5, 2025
* Add test to ensure conda does not allow for mixing input filetypes

* Validate that input files are of the same type

* Add validation to prevent mixing different environment file formats

* Improve error handling when mixing environment file types in conda create

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Consolidate environment file validation and test file organization

* Delete unneeded files.

* Add validation check to conda env command as well.

* Simplify environment file type validation by removing unnecessary variable

* No need to test for this, since `env create --file` is a singular option

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* remove unused EnvironmentFileTypeMismatchError import in test_create.py

* Move get_environment_specifier_name method and add unit tests

* Fix linting errors

* Fix rebase error

* Add news

---------

Co-authored-by: Jannis Leidel <jannis@leidel.info>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@soapy1 soapy1 mentioned this pull request Jun 5, 2025
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed [bot] added once the contributor has signed the CLA
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants