ndexbio / ndex2-client

NDEx2 Client
BSD 3-Clause "New" or "Revised" License
6 stars 6 forks source link

Avoid using `id` as variable name #55

Closed zachary822 closed 4 years ago

zachary822 commented 5 years ago

https://github.com/ndexbio/ndex2-client/blob/04af1f0a2e3afe419f4a5844e55b346f62183945/ndex2/nice_cx_network.py#L171

Considering id is a built in function. I would avoid using it as a variable name throughout the code.

coleslaw481 commented 5 years ago

You are right, that is bad form. I'll fix the internal methods asap, but the public facing ones will need a bit more time since I'll have to give deprecation warnings and a roadmap for future versions (also need to make a roadmap) as to when they will go away.

coleslaw481 commented 4 years ago

Looks like the following public methods (no __ prefix) have an id parameter that needs to be changed:

ndex2/nice_cx_network.py:    def add_citation(self, id, title=None, contributor=None, identifier=None, type=None, description=None, attributes=None):
ndex2/nice_cx_network.py:    def add_edge(self, id=None, source=None, target=None, interaction=None):
ndex2/nice_cx_network.py:    def add_node(self, id=None, name=None, represents=None):
ndex2/nice_cx_network.py:    def add_support(self, id=None, text=None, citation_id=None, attributes=None, props=None):
ndex2/niceCxInterface.py:    def create_edge(self, id=None, edge_source=None, edge_target=None, edge_interaction=None, cx_fragment=None):
ndex2/niceCxInterface.py:    def create_node(self, id=None, node_name=None, node_represents=None, cx_fragment=None):
ndex2cx/nice_cx_builder.py:    def add_node(self, name=None, represents=None, id=None, data_type=None, map_node_ids=False):
ndex2cx/nice_cx_builder.py:    def add_edge(self, source=None, target=None, interaction=None, id=None):
ndex2/nice_cx_network.py:    def get_edge_attributes_by_id(self, id):
ndex2/niceCxInterface.py:    def get_edge_attributes_by_id(self, id):
coleslaw481 commented 4 years ago

Fixed in v4.0.0 branch with breaking changes.

coleslaw481 commented 4 years ago

Closing to denote this has been fixed