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

Stop polluting the namespace! #154

Closed
ceball opened this issue Jun 12, 2017 · 8 comments
Closed

Stop polluting the namespace! #154

ceball opened this issue Jun 12, 2017 · 8 comments
Labels
API breaking Affects the api in a non bug fixy way/consider version or name change component: "parameterized" help me name this :) status: discussion Discussion. Not yet a specific feature/bug. Likely to result in multiple PRs/issues.
Milestone

Comments

@ceball
Copy link
Contributor

ceball commented Jun 12, 2017

E.g. could move nearly all attributes and methods of Parameterized into a sub-object.

Will need to provide a migration path.

@jlstevens
Copy link
Contributor

Any reason this sub-object can't be on a param object?

In Parameterized, you would have self.param = SomeNamespace(...). I think once it is bound as an attribute on a class or object, there won't be issue with this name clashing with param the module.

@jbednar
Copy link
Member

jbednar commented Aug 30, 2017

I thought about that too, but decided that it might make the docs confusing, because people wouldn't be able to distinguish easily between param.something_defined_at_the_module_level and param.something_defined_in_the_subobject. In code I think it's fine, but because everything will be on this subobject, we'll want a short way to refer to it in docs, and the obvious param as the short name (as opposed to Parameterized.param) would be unsuitable. If only param had some obvious short name we could use here for the subobject, but it's already a shortening and does not seem to. Just p is problematic, since we're already using that by convention. Maybe self.prm?

@jlstevens
Copy link
Contributor

I'm not too sure that is an issue - it should be fairly easy to tell the module and sub-object apart as the module is used for class which are capitalized (e.g param.Parameterized, param.Integer etc) and the sub-object is holding methods which are lowercase (set_param, warning etc).

There are a few top-level functions (which could be moved into a util file?) but I think the vast majority of normal param usage falls into the two categories I just mentioned. I don't think there would be much confusion in practice and any other name (e.g prm) just seems a lot less nice to me.

@jbednar
Copy link
Member

jbednar commented Sep 1, 2017

Fine by me, then.

@jlstevens
Copy link
Contributor

I suppose we could make a branch, try it out and see how much we like it?

Trying it out is the best way for us to figure out how happy we are with any particular suggestion.

@jbednar
Copy link
Member

jbednar commented Sep 1, 2017

Is trying it out as simple as doing self.param = self.__dict__ in the Parameterized constructor? That won't clean up the namespace, but it seems like it would let you see what using the new way would be like.

@ceball ceball added API breaking Affects the api in a non bug fixy way/consider version or name change component: "parameterized" help me name this :) labels Apr 13, 2020
@ceball ceball removed this from the 2.0 milestone Apr 13, 2020
@ceball ceball added the status: discussion Discussion. Not yet a specific feature/bug. Likely to result in multiple PRs/issues. label Apr 13, 2020
@jbednar jbednar mentioned this issue May 18, 2020
@jbednar jbednar added this to the 2.0 milestone Aug 10, 2020
@jbednar
Copy link
Member

jbednar commented Aug 10, 2020

Nearly done already, but needs the actual namespace cleanup to be done, which is a breaking change that is waiting on param 2.0.

@jbednar
Copy link
Member

jbednar commented Sep 25, 2021

Covered under #233.

@jbednar jbednar closed this as completed Sep 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API breaking Affects the api in a non bug fixy way/consider version or name change component: "parameterized" help me name this :) status: discussion Discussion. Not yet a specific feature/bug. Likely to result in multiple PRs/issues.
Projects
None yet
Development

No branches or pull requests

3 participants