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

1358 rename Data to data #1421

Closed
wants to merge 11 commits into from
Closed

Conversation

postelrich
Copy link
Contributor

closes #1358

rename Data to data and add deprecation warning to Data.

@@ -310,3 +310,13 @@ def __get__(self, instance, owner):
return self

return self._f(instance)


class BlazeDeprecationWarning(DeprecationWarning):
Copy link
Member

Choose a reason for hiding this comment

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

is there a reason to make these classes rather than just using the superclass directly?

Copy link
Member

Choose a reason for hiding this comment

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

We originally thought it would be good to have a subclass to make it easily filterable, but now I'm with you, and see no really good reason not to just use DeprecationWarning directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea was to so that we can set warnings.simplefilter to only display the warning once but I couldn't get it to work. I created the derived class so that I wouldn't effect how other warnings are displayed, especially since deprecation warnings are ignored by default.

@llllllllll
Copy link
Member

How do people feel about the name, bound_symbol or something? data feels too close to what resource is.

@kwmsmith
Copy link
Member

How do people feel about the name, bound_symbol or something? data feels too close to what resource is.

As you know, we discussed this at length in another issue thread.

We bikeshedded the naming question again among 4-5 people before starting this PR. We decided to stay with data:

  • It's a very common end-user entry point, so we want to keep it short;
  • Given the target end user--Data Scientist, Business Analyst, Data Analyst, etc.--bound_symbol was thought to be too far removed from their working vocabulary;
  • data, compute, and expression are the three main concepts we use when talking about blaze, so it's a big plus to have those three concepts reflected in the API directly.

@kwmsmith kwmsmith added this to the 0.10 milestone Feb 23, 2016
@postelrich
Copy link
Contributor Author

@llllllllll I agree that datais too generic of a name (especially as part of a data science library/distribution) and not descriptive enough of what it's doing. But I also agree with @kwmsmith against naming it something too technical.

At the risk of starting another lengthy discussion, I propose that if we were to change the name that it be simple. So something like blaze.ignite because you're firing up a powerful connection to your data. Or really simple like blaze.on or blaze.open.

But we shouldn't delay progress, rather just keep the discussion open and make a decision before we get to a 1.0.

@llllllllll
Copy link
Member

@kwmsmith Sorry, I forgot that we had come to a conclusion in the other thread.

@kwmsmith
Copy link
Member

Closing this PR, see #1431.

@kwmsmith kwmsmith closed this Feb 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Data -> data and other API inconsistencies
3 participants