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

remove keras_templates imports from models __init__ #82

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

sbuergers
Copy link
Collaborator

@sbuergers sbuergers commented Dec 22, 2023

#83

in models and metrics we imported keras functions from keras_templates in the __init__.py, so even if users of SAM want to not use keras they need to have this rather big dependency installed if they want to use, say, a Lasso model.

This PR needs some more work and most of all testing so that the parts of SAM that should not depend on KERAS or TENSORFLOW have all tests passing without it being installed.

@sbuergers sbuergers added the Type: Improvement Any technical improvement without changing functionality label Dec 22, 2023
@sbuergers sbuergers self-assigned this Dec 22, 2023
@@ -94,7 +94,7 @@ def _index_has_constant_increase(df: pd.DataFrame):
else:
return False

if type(df.index) == pd.core.indexes.datetimes.DatetimeIndex and (
if type(df.index) is pd.core.indexes.datetimes.DatetimeIndex and (
Copy link
Collaborator

Choose a reason for hiding this comment

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

isinstance(df.index, pd.core.indexes.datetimes.DatetimeIndex)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ruben preferred the type() is syntax. I don't mind either way. I think the situations in which they differ almost never occur, and should not in this case, right?

@sbuergers sbuergers marked this pull request as draft December 22, 2023 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Improvement Any technical improvement without changing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants