Skip to content

Conversation

alixdamman
Copy link
Collaborator

Waiting for PR #809 to be merged before to finish this PR.

@alixdamman alixdamman added this to the 0.32 milestone Oct 2, 2019
@alixdamman alixdamman force-pushed the 611_rename_LArray_class_as_Array branch from a1ead7e to 667e62b Compare October 8, 2019 09:58
@alixdamman
Copy link
Collaborator Author

alixdamman commented Oct 8, 2019

The:

are the most important commits to review.

@alixdamman alixdamman requested a review from gdementen October 9, 2019 08:58
Copy link
Contributor

@gdementen gdementen left a comment

Choose a reason for hiding this comment

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

LGTM

class LArray(Array):
def __init__(self, *args, **kwargs):
warnings.warn("LArray has been renamed as Array.", FutureWarning, stacklevel=2)
Array.__init__(self, *args, **kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

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

can't we use LArray = renamed_to(Array, 'LArray') instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

rename_to return a function and not a class. In addition, the printed warning message would have been LArray() is deprecated. Use Array() instead, i.e. with ( ) after class names.

Copy link
Contributor

Choose a reason for hiding this comment

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

Calling a function and a class is essentially the same thing in Python (they are both callables) and I don't see the problem with that warning message.

@alixdamman alixdamman force-pushed the 611_rename_LArray_class_as_Array branch from 9256ef7 to 29c0d76 Compare October 9, 2019 09:39
@alixdamman alixdamman merged commit 5ab8316 into larray-project:master Oct 9, 2019
@alixdamman alixdamman deleted the 611_rename_LArray_class_as_Array branch October 9, 2019 09:45
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