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

Do not add pretty_errors as a requirement #45

Merged
merged 1 commit into from
May 14, 2024
Merged

Conversation

183amir
Copy link
Contributor

@183amir 183amir commented May 14, 2024

pretty_errors is only a debug requirement https://github.com/Lightning-AI/torchmetrics/blob/bfcd4b0e5bfa1256ef9c142956a86df57542a668/requirements/debug.txt installing this package will change the traceback output of python as soon as torchmetrics is imported: https://github.com/Lightning-AI/torchmetrics/blob/bfcd4b0e5bfa1256ef9c142956a86df57542a668/src/torchmetrics/__init__.py#L18. It was so difficult to find out what was changing tracebacks in my script.

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

pretty_errors is only a debug requirement https://github.com/Lightning-AI/torchmetrics/blob/bfcd4b0e5bfa1256ef9c142956a86df57542a668/requirements/debug.txt
installing this package will change the traceback output of python as soon as torchmetrics is imported. It was so difficult to find out what was changing tracebacks in my script.
@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@183amir
Copy link
Contributor Author

183amir commented May 14, 2024

@conda-forge-admin, please rerender

Copy link
Contributor

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/torchmetrics-feedstock/actions/runs/9086188030.

@Borda Borda merged commit 485aaab into conda-forge:main May 14, 2024
3 checks passed
@RaulPPelaez
Copy link

This broke a feedstock build that uses torchmetrics: conda-forge/torchmd-net-feedstock#24
I get this error when testing:

import: 'torchmdnet'
import: 'torchmdnet'
+ pip check
torchmetrics 1.4.0 requires pretty-errors, which is not installed.

@183amir
Copy link
Contributor Author

183amir commented May 15, 2024

I suggest removing this dependency from torch metrics completely. You don't want your Python errors to randomly change without knowing what's going on. It's extremely annoying and very difficult to find the culprit.

@RaulPPelaez
Copy link

This seems indeed like a strange dependency for torchmetrics. The error seems to be in the pip scripts, since it is pip check complaining.
Perhaps the problem is this PR not being in a release yet? Lightning-AI/torchmetrics#2527

@Borda
Copy link
Member

Borda commented May 15, 2024

Perhaps the problem is this PR not being in a release yet? Lightning-AI/torchmetrics#2527

That was not released yet... But we can push it if there is such issue...

@RaulPPelaez
Copy link

I reckon it should be at least added as a patch in this feedstock, mine is not the only feedstock that's going to start requiring pretty_errors as an explicit dependency otherwise.

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