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

Deprecation #19

Merged
merged 7 commits into from
Feb 29, 2024
Merged

Deprecation #19

merged 7 commits into from
Feb 29, 2024

Conversation

charlie-becker
Copy link
Contributor

  1. Added a deprecated module with the old models, losses, and callbacks.
  2. Modified losses.py to remove old losses and add new ones
  3. Modified callbacks.py to remove old ones and add new ones

@djgagne
Copy link
Collaborator

djgagne commented Feb 29, 2024

The CI had the following error

ImportError while importing test module '/home/runner/work/miles-guess/miles-guess/mlguess/tests/test_models.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../micromamba/envs/guess/lib/python3.10/importlib/__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
mlguess/tests/test_models.py:9: in <module>
    from mlguess.keras.models import BaseRegressor as RegressorDNN
mlguess/keras/models.py:13: in <module>
    from mlguess.keras.losses import EvidentialRegressionLoss, EvidentialRegressionCoupledLoss, gaussian_nll
E   ImportError: cannot import name 'EvidentialRegressionLoss' from 'mlguess.keras.losses' (/home/runner/work/miles-guess/miles-guess/mlguess/keras/losses.py)

@djgagne
Copy link
Collaborator

djgagne commented Feb 29, 2024

@charlie-becker it looks like you need to update your tests to use the new losses and remove or refactor the imports for the old losses.

@charlie-becker
Copy link
Contributor Author

Model classes have been fully refactored. I have removed the tests for the non-evidential models (as they have been moved to the deprecated module).

.predict() functions in both of the new model classes (classification and regression) now include a return_uncertainties= argument (bool).

This should thoroughly documented in the README which I can accomplish in another PR.

Copy link
Collaborator

@djgagne djgagne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Merging.

@djgagne djgagne merged commit c11779c into main Feb 29, 2024
4 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