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

Is there any interest in supporting IPNetwork as well? #40

Open
ddutt opened this issue Jun 18, 2019 · 17 comments
Open

Is there any interest in supporting IPNetwork as well? #40

ddutt opened this issue Jun 18, 2019 · 17 comments

Comments

@ddutt
Copy link

ddutt commented Jun 18, 2019

Hi,

I'm interested in IPNetwork as well. Is there any work being done on this already? If not, is there any interest in supporting it?

Thanks,

Dinesh

@TomAugspurger
Copy link
Contributor

TomAugspurger commented Jun 18, 2019 via email

@ddutt
Copy link
Author

ddutt commented Jun 18, 2019

Cool. Any recommendations on providing the pull request provided I get to doing it?

Dinesh

@ddutt
Copy link
Author

ddutt commented Jun 18, 2019

It seems best to create a new type called IPNetworkArray rather than fiddle with IPArray. Would you agree?

@TomAugspurger
Copy link
Contributor

Yes, a separate ExtensionDtype and ExtensionArray makes the most sense.

Will the storage be essentially the same? An ndarray with a record dtype for the hi and lo bits?

I think you'll want to inherit from NumPyBackedExtensionArrayMixin: https://github.com/ContinuumIO/cyberpandas/blob/master/cyberpandas/base.py. I wrote that as a prototype for pandas, so it's a bit convoluted, but may be useful.

I suspect you find some duplicate functionality with IPArray. I wouldn't worry about deduplicating things too much at first.

@ddutt
Copy link
Author

ddutt commented Jun 18, 2019

I was planning to store the thing as a CIDR string, /prefixlen. Would that work? I haven't written an extensions to Pandas dtypes before.

@TomAugspurger
Copy link
Contributor

TomAugspurger commented Jun 18, 2019 via email

@seibert
Copy link
Collaborator

seibert commented Jun 18, 2019

It would be convenient for a number of operations if the representation of the network could be fixed width. Packed variable length strings are likely to make things complicated.

@ddutt
Copy link
Author

ddutt commented Jun 19, 2019

So I have a prelim implementation working, but not sure of its efficiency. I stored the network as an IPv4Network or IPv6Network object itself instead of trying to do a string or any other fancy implementation. For things like is_ipv4, the IPArray approach is much faster and for things like is_multicast, is_link_local, the approach I've taken seems faster from a simple %timeit on a few things perspective.

I'd like some eyes on what I've done to see if there are changes to make the thing better. What do you suggest? I still need a lot of cleaning up if all looks good.

Dinesh

@ddutt
Copy link
Author

ddutt commented Jun 19, 2019

I've got df.a.astype('ipnetwork'), df.query('a.ipnet.is_link_local') etc. working

Dinesh

@TomAugspurger
Copy link
Contributor

TomAugspurger commented Jun 19, 2019 via email

@ddutt
Copy link
Author

ddutt commented Jun 19, 2019

Thanks Tom. Since you won't have time for a bit, let me clean it up and ensure the data is private for now. I'll look at what I can do (maybe add a third byte which is the prefixlen to your ip address model) to make this more efficient. Is Cython a possibility here? I'll send a PR after I'm comfortable its cleaned up.

Does that make sense?

Dinesh

@TomAugspurger
Copy link
Contributor

TomAugspurger commented Jun 19, 2019 via email

@ddutt
Copy link
Author

ddutt commented Jun 20, 2019

I've created a pull request. I've tried to follow the model you've followed for IPArray in terms of where the different functions live. I've also added support for max/min besides support for supernet_of and subnet_of.

@giganteous
Copy link

Hello, is there any progress on this issue?

@mzpqnxow
Copy link

@ddutt , @TomAugspurger did this go anywhere? I have a use-case for this and was curious if it's still "a thing"

@TomAugspurger
Copy link
Contributor

Not really, I'm don't have time to work on cyberpandas these days.

@ddutt
Copy link
Author

ddutt commented Sep 22, 2021

I don't have time to work on cyberpandas either. I found other ways to work around it. But if you have some code, I'm happy to test it.

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

No branches or pull requests

5 participants