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.

Sign up for GitHub

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

Jump to bottom

int and float uids no longer supported #99

Open
hackl opened this issue May 4, 2021 · 2 comments
Open

int and float uids no longer supported #99

hackl opened this issue May 4, 2021 · 2 comments
Labels
discussion Discussions on pathpy development and scientific approaches

Comments

@hackl
Copy link
Collaborator

hackl commented May 4, 2021

With the new path implementation, the question is if we want to support int and float values as uid. Currently only str values are supported.

In the previous version, int values were converted to str values however this is not consistent with the __getitem__ operator. i.e.

>>> # old version
>>> nodes = NodeCollection()
>>> nodes.add(1)
>>> 1 in nodes
False
>>> '1' in nodes
True

furthermore, int and float values are also used in the TemporalNetwork to access certain timestamps.

Therefore, I would suggest limiting uids to str values and use int and float for other purposes.
@hackl hackl added the discussion Discussions on pathpy development and scientific approaches label May 4, 2021
@IngoScholtes
Copy link
Member

I think it would be useful to support any hashable uid type

@IngoScholtes
Copy link
Member

IngoScholtes commented Sep 17, 2021 •
edited
Loading

Adding to my previous comment, the current solution of automatically converting uids is not ideal. Consider this code:

n = pp.Network()
n.add_edge(0, 1)
print(n.nodes[0])
print(n.nodes['0'])

Due to the automatic conversion to string (which is probably very costly) it is not immediately clear, why the lookup of node 0 fails. This will lead to hard to understand bugs for the users.

Alos, the conversion is not applied consistently, i.e. one cannot add edges using indices of np.ndarrays.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
discussion Discussions on pathpy development and scientific approaches
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@IngoScholtes @hackl