Remove optional From Dependency Info in Internal Code
#389
Conversation
✅ Deploy Preview for conda-lock ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
srilman
force-pushed
the
remove-optional-flag
branch
from
8040d20 to
b372c6a
Compare
srilman
force-pushed
the
remove-optional-flag
branch
from
b372c6a to
5e96205
Compare
| @@ -542,7 +542,7 @@ def render_lockfile_for_platform( # noqa: C901 | |||
| lockfile.toposort_inplace() | |||
|
|
|||
| for p in lockfile.package: | |||
| if p.platform == platform and ((not p.optional) or (p.category in categories)): | |||
| if p.platform == platform and p.category in categories: | |||
There was a problem hiding this comment.
Note to self: not p.optional ⇒ p.category == "main".
Since "main" in categories, this is redundant and can be deleted.
Therefore, this is logically sound.
| @@ -95,6 +93,9 @@ def parse_conda_lock_file( | |||
| if not (isinstance(version, int) and version <= Lockfile.version): | |||
| raise ValueError(f"{path} has unknown version {version}") | |||
|
|
|||
| for p in content["package"]: | |||
| del p["optional"] | |||
There was a problem hiding this comment.
Note to self: we need to delete the optional key in order to parse this as a StrictModel.
|
Or does it make more sense to merge #390 all at once, once I've completed that? |
|
Sorry, I seem to have been confused. |
|
@maresb Resolving conflicts now ... |
updates: - [github.com/pre-commit/mirrors-mypy: v1.0.1 → v1.1.1](pre-commit/mirrors-mypy@v1.0.1...v1.1.1)
srilman
force-pushed
the
remove-optional-flag
branch
from
d0d86be to
5ee8289
Compare
|
Merged with main, everything should be good! |
|
Looking great, thanks!!! As we wait for the Windows test to finish, can we rebase #390 on this? |
| "metadata": json.loads( | ||
| content.metadata.json( |
There was a problem hiding this comment.
I just noticed that in #390 you have here instead "metadata": content.metadata.dict( which seems more elegant. Is this an accident, or does this cover some edge case?
There was a problem hiding this comment.
I believe there was an issue when encoding content.metadata as a dict due to a special class / container that was used in the metadata (was it a frozenset?). Let me check for certain.
|
The tests are passing. Let's pick up on #389 (comment) with #390. I have to stop for tonight. |
This PR removes any references to an
optionalflag or option from the entire codebase. This builds on top of PR #388, so make sure to get that one merged first. At that point, there is an implicit invariant for all dependencies; the dependency is optional ifcategory != "main". Therefore, storing the optional value just makes the code unnecessarily complex.Note that this does not affect how source files are parsed or the produced lock file in any way. The step that writes the lockfile out will insert
optional: trueoroptional: falseflags depending on the category.