4 ms·

Confession: I hate that. Hate it. It does fiddly magic at package installation time and then there’s zero indication that anything’s happened. It’s magic. When

Confession: I hate that. Hate it. It does fiddly magic at package installation time and then there’s zero indication that anything’s happened. It’s magic. When I’m looking at the end result, there’s no hint whatsoever at how those new URLs are being handled. Did Python somehow learn csvbase://? Pandas? Where’s that coming from? How do I fix it if it breaks? Please don’t do that. Edit: Explicit patching would be just fine, like: from my_project import csvbase_patch csvbase_patch() p = pandas.pd(“csvbase://…”) Then there’s a giant indicator inside the same module that there’s magic happening. If I cmd-f “csvbase”, the magic string in the URL, I’ll stumble across that patch function. Then I can jump to its definition to see what’s happening. This is Python. Hidden monkeypatching isn’t how we do things.

corytheboyd 3y ago
Please, for the love of god, listen to this commenter. I’m coming from the Ruby side of things, and hidden monkeypatching will be the death of us all.
- actionfromafar 3y ago
  I was going to say "this reminds me of Ruby". :-D
- kstrauser 3y ago
  When I wrote “This is Python”, I resisted the urge to finish with “not Ruby”.
sloowm 3y ago
Thanks for the edit. I agree it should be explicit and then I actually think it's quite a nice solution.
brigadier132 3y ago
It's absolutely terrible. My first job out of college I was basically only fixing bugs in a python codebase. 99% of the bugs were from monkey patching and null pointer exceptions where people were trying to access properties on objects in places they were not instantiated yet. People that write code like this should be forced into bug jail like i was for a year.
philomath_mn 3y ago
I generally agree: explicit is better than implicit, magic can be dangerous. But the author is not doing any monkeypatching or changing how pandas works. He is using fsspec [0] to create a filesystem interface that pulls data from his site. fsspec appears to be somewhat standard since pandas, polars, Dask, and other libraries use it. As soon as I understand that fsspec exists and this library uses it, there is no more magic. I would prefer if the specification of which fsspec to use were not embedded as part of a string, but overall this approach seems pretty reasonable. [0] https://filesystem-spec.readthedocs.io/en/latest/ https://filesystem-spec.readthedocs.io/en/latest/
- kstrauser 3y ago
  It’s still magic, just implemented with a shared crystal ball. If I learned about fsspec at 2AM during a prod outage via reading tracebacks instead of reading code, I’d be a font of creative profanity. The idea is nice! I’m not saying fsspec is bad. It’s not. It’s neat! I just strongly abhor the idea that pip installing a package changes runtime behavior whether that packages is ever even imported.
  - philomath_mn 3y ago
    Yeah, I'd prefer the pandas call look like ``` from csvbase import CSVBASE_FS pd.read_csv('//calpaterson/onion-vox-pops', fsspec=CSVBASE_FS) ```
    - kstrauser 3y ago
      I vastly, hugely, enormously prefer that pattern. It’s not as pretty as the blog post, but it’s so explicit and easy to understand at a glance. If I’d never heard of csvbase or fsspec before seeing that code, I’d still know what to look for and how to debug it. At 2PM when I’m well rested, caffeinated, and alert, the original code is clever and lovely. At 2AM, I want this kind of easy to understand explicitness.
      - philomath_mn 3y ago
lolinder 3y ago
> This is Python. Hidden monkeypatching isn’t how we do things. The author didn't make this pattern up, it's how fsspec officially recommends implementing backends [0]. Given how widely used fsspec is I think it's fair to say that hidden monkeypatching is how we do things... sometimes. [0] https://filesystem-spec.readthedocs.io/en/latest/developer.html#implementing-a-backend https://filesystem-spec.readthedocs.io/en/latest/developer.h...
- kuchenbecker 3y ago
  Monkeypatching, dll injection, Jar Classpath - Huge sources of pain. It Is how it's done but in certain contexts any benefit is immediately outweighed as the bugs start accumulating.
  - lolinder 3y ago
    Right, in certain contexts (I would even say most contexts) it can be a problem. However, it's not obvious to me that this—a well-defined interface for injecting plugins that's specified by a library that functions as low-level infrastructure—is one of those contexts.
    - kuchenbecker 3y ago
      It needs to be scoped to the dep so conflicting deps have independent dependencies. If foo 1.1 takes a dep on Bar 1.2, and baz 2.3 takes a dep on Bar 1.4, both Bar contexts are tested with their respective libraries but forcing Bar to a particular global version can have problems and both versions of Bar are needed to have tested behavior. Examples mentioned and OP change the global behavior vs proper dependency management.
threePointFive 3y ago
I like Go's approach to this same idea. The standard database/sql library provides the standard API and the individual database drivers implement their own backend. You can use the URI connection string for your database (ie. postgres://...) though only after including the driver in your file's imports. There's even the idiomatic underscore prefix to the package import to note that you're only importing it for how it's presence affects another package. Unfortunately there's no way to sway which package you're affecting, but its still better than hidden changes.
- didntcheck 3y ago
  So basically JDBC? :) I think a similar approach is used with the crypto provider API and some others IIRC (in JDBC) you also used to have to do `Class.forName("name.of.it")` somewhere before trying to do any DB access, to ensure that the static initializers had actually run, but I don't believe it's necessary anymore (And then of course you have Spring Boot autoconfiguring which is another level of magic up, using automatic subclassing and proxy injection to add things like transaction management. And then you can get into proper classloader hackery)
  - Groxx 3y ago
    Essentially, yes. And you need to register before use in Go too since reflection is far more limited in Go - often this is done at init time, so you just import the package that does the registration [somehow], but ultimately you just have to do it before you use something: https://pkg.go.dev/database/sql#Register https://pkg.go.dev/database/sql#Register The "data source name" string when connecting is... basically a JDBC connection string, and some adapters use exactly that iirc, but it's fundamentally an unstructured string that just serves the same purpose. Plugins can use anything they like, and style varies.
throwitaway222 3y ago
Disagree here. JDBC, ODBC, standards are great. There are old tools that can connect to generic databases because of said standards, you just need the right JDBC URL, or perhaps in Windows, ODBC. It's what makes interoperability work amazing things without having to recompile your code. "hidden monkeypatching" is essentially how all of Java works.
- kstrauser 3y ago
  Lacking much Java experience, I’ll accept this as true. If so, that’s another reason I’m uninterested in Java. Magic behavior changes based on the prefix of a string passed into a function doesn’t appeal to me at all. The Python equivalent for a database might look like (typed from memory): from psycopg import connect conn = connect(host, username, password, dbname) Where connect() returns a database adapter object with DB-API methods. When something breaks, I can see which DB code is involved. It’s right there in the import. My IDE doesn’t have to parse strings to know which code path subsequent method calls are following.
- nightpool 3y ago
  And it worked out super well for everybody in 2021 :)
valenterry 3y ago
> Explicit patching would be just fine Not fine. Just less horrible.
deleted 3y ago
[deleted]
mattpallissard 3y ago
> Hidden monkeypatching isn’t how we do things. The original authors of several codebases I've been saddled with over the years would disagree with you. This is Python and you a peasant. Yes most of the time the snake along the road is a snake, but occasionally it's a Basilisk waiting to destroy you with it's gaze. Knowing this, you the weary traveler, carry inconvenient tools such as mirrors to protect yourself and are well versed in superstition and arcane dark arts. You never let your guard down, because yes there is magic afoot.
- kstrauser 3y ago
  It's not like "Zen of Python" is an inviolable law, but things that break it are not, in my opinion, "Pythonic". The top 2 aphorisms are: "Beautiful is better than ugly." I think this is ugly. It's visually appealing, but my editor doesn't know that "csvbase" is a magic symbol that selects the code path to follow. That deep ugliness outweighs any skin-deep cleverness. "Explicit is better than implicit." That sums it up. This is implicit. `pip install foo` changing runtime behavior of packages that never import foo is as implicit as it gets. From a developer's and a security engineer's point of view, I loathe that code is running simply because it exists. It's possible to do these things. I contend that it's not Pythonic, and we should not be doing them.
everforward 3y ago
> This is Python. Hidden monkeypatching isn’t how we do things. It kind of is, though. This isn’t the first time I’ve seen this; off the top of my head, I believe the Stackdriver libraries do something similar but to the stdlib logging library. The fact that it’s possible is also a tacit endorsement of doing things this way. Many languages just don’t permit adding to or modifying a namespace. I would argue monkey-patching is a bad pattern in general, regardless of whether it’s manual or automatic. IMO, fsspec should have a private package-level variable with a list of these adapters, and expose a ‘fsspec.register_adapter(adapter)’ method to add things to it. I don’t see a need or use in patching here. It just seems fraught with issues that could be avoided with an adapter registry. Testing seems easier too; I can’t imagine the pain of unit testing a bunch of adapters if they each try to modify the fsspec package directly.
pantsforbirds 3y ago
Do you get upset when you see one of create_engine("postgresql://"), create_engine("mysql://"), or create_engine("sqlite://")? The author is just adding a new backend for a common protocol in the data world. Pandas, Polars, Dask, DuckDB, etc. all support this protocol and the type of people who want to access a dedicated CSV data archive would probably much rather keep their current client API and just add a new connection URI string vs. adding an entire client API for just one data source (or dealing with making requests and passing the data into the dataframe).
- kstrauser 3y ago
  I didn't say it was uncommon, just that it's a bad idea. There's no need whatsoever for a separate client API. There could be a convention like: from csvbase import loader as csvloader df = pd.read_csv(csvloader("calpaterson/onion-vox-pops")) The user wouldn't have to know anything but what to import to fetch a certain thing, and it's explicit about what's coming from there. There's also less risk of mistyping the URL string ("oops, I just typed cvsbase and accidentally loaded a list of CVS drugstores"), and code completion can tell you that csvloader() fetches things through the csvbase module. Cons: - It takes 5 seconds longer to type, one time. Pros: - You can tell what the code does at a glance.
- mekoka 3y ago
  You're missing the point. It's not about the use of bespoke URIs. This is the debate on "convention over configuration" revisited. Conventions are cute and always look clever while you're reading documentation. But they're a pain during maintenance, as they force others who don't know them to first go read the doc, instead of simply inferring from the previously researched and clearly stated declarations how all the wires connect. When I see `pd.read_csv("csvbase://")` during debug, I wonder how pandas knows to speak to csvbase (as the article anticipates). Nothing is imported. Nothing is configured. Things just speak to one another. So, can I also call pd.read_csv("other_csv_server://") like this? When I replace pandas with koalas, will koalas.read_csv("csvbase://") also work? How the wires connect between pandas and csvbase is hidden. Unless you know that the two are obeying some implicit lower layer (the fsspec standard), this becomes a mystery. Mysteries are the last thing you want when debugging. I don't know which `create_engine()` function you're alluding to. The one I know and have used comes from SQLAlchemy. How it works has always been obvious. I've never seen any mention of fsspec. I looked at its code and it's predictably just a convenient syntax to specify connection information in a single string. The string is simply parsed to extract connection attributes, which are then relayed to the lower DBAPI. There's no mystery involved.
calpaterson 3y ago
I'm sorry you're not a fan :) I do take your point - to an extent. I think the subject you're touching on is basically that of configuration. Configuration is what allows you to change the behaviour of code without making a code change. Configuration is of course very really powerful for good and evil. Someone below mentions ODBC which, yes, 100% is a great example. But the one that sticks in my mind is resolv.conf. I would contend (:)) that csvbase-client is not doing "fiddly magic at package installation time" but supplying configuration for a url scheme. In much the same way that installing boto3 does for using the `s3://` url scheme. "Magic" in my book would be monkeypatching the methods of other libs. I would hate to write `csvbase_patch` and have it execute on the import of a specific module. Like you, I prefer to live in a low-magic fantasy world.
- kstrauser 3y ago
  To be super clear, I'm not a fan of this specific thing. The rest of csvbase may be the best thing ever! This kind of configuration feels very magical in that it's completely behind the scenes. There's nothing in code I can look at that says "this URL should be handed to this adapter". There's no env var that gets slurped in. No .env file that's parsed. There's nothing checked into git that tells me how that URL scheme could possibly work, except the existence of a package in pyproject.toml -- one that's not even imported, just present. That makes it stateful in a bad way. If `poetry install` (or `pip install -r requirements.txt` or whatever) didn't complete, the module using it will still load and run up until the point that it crashes because that URL doesn't load. If I ran an automated process to prune unused modules from poetry/pip, suddenly behavior changes. I don't like that idea one bit.

corytheboyd 3y ago

Please, for the love of god, listen to this commenter. I’m coming from the Ruby side of things, and hidden monkeypatching will be the death of us all.

actionfromafar 3y ago
I was going to say "this reminds me of Ruby". :-D
kstrauser 3y ago
When I wrote “This is Python”, I resisted the urge to finish with “not Ruby”.

sloowm 3y ago

Thanks for the edit. I agree it should be explicit and then I actually think it's quite a nice solution.

brigadier132 3y ago

It's absolutely terrible. My first job out of college I was basically only fixing bugs in a python codebase. 99% of the bugs were from monkey patching and null pointer exceptions where people were trying to access properties on objects in places they were not instantiated yet. People that write code like this should be forced into bug jail like i was for a year.

philomath_mn 3y ago

I generally agree: explicit is better than implicit, magic can be dangerous. But the author is not doing any monkeypatching or changing how pandas works. He is using fsspec [0] to create a filesystem interface that pulls data from his site. fsspec appears to be somewhat standard since pandas, polars, Dask, and other libraries use it. As soon as I understand that fsspec exists and this library uses it, there is no more magic. I would prefer if the specification of which fsspec to use were not embedded as part of a string, but overall this approach seems pretty reasonable. [0] https://filesystem-spec.readthedocs.io/en/latest/ https://filesystem-spec.readthedocs.io/en/latest/

kstrauser 3y ago
It’s still magic, just implemented with a shared crystal ball. If I learned about fsspec at 2AM during a prod outage via reading tracebacks instead of reading code, I’d be a font of creative profanity. The idea is nice! I’m not saying fsspec is bad. It’s not. It’s neat! I just strongly abhor the idea that pip installing a package changes runtime behavior whether that packages is ever even imported.
- philomath_mn 3y ago
  Yeah, I'd prefer the pandas call look like ``` from csvbase import CSVBASE_FS pd.read_csv('//calpaterson/onion-vox-pops', fsspec=CSVBASE_FS) ```
  - kstrauser 3y ago
    I vastly, hugely, enormously prefer that pattern. It’s not as pretty as the blog post, but it’s so explicit and easy to understand at a glance. If I’d never heard of csvbase or fsspec before seeing that code, I’d still know what to look for and how to debug it. At 2PM when I’m well rested, caffeinated, and alert, the original code is clever and lovely. At 2AM, I want this kind of easy to understand explicitness.
    - philomath_mn 3y ago

lolinder 3y ago

> This is Python. Hidden monkeypatching isn’t how we do things. The author didn't make this pattern up, it's how fsspec officially recommends implementing backends [0]. Given how widely used fsspec is I think it's fair to say that hidden monkeypatching is how we do things... sometimes. [0] https://filesystem-spec.readthedocs.io/en/latest/developer.html#implementing-a-backend https://filesystem-spec.readthedocs.io/en/latest/developer.h...

kuchenbecker 3y ago
Monkeypatching, dll injection, Jar Classpath - Huge sources of pain. It Is how it's done but in certain contexts any benefit is immediately outweighed as the bugs start accumulating.
- lolinder 3y ago
  Right, in certain contexts (I would even say most contexts) it can be a problem. However, it's not obvious to me that this—a well-defined interface for injecting plugins that's specified by a library that functions as low-level infrastructure—is one of those contexts.
  - kuchenbecker 3y ago
    It needs to be scoped to the dep so conflicting deps have independent dependencies. If foo 1.1 takes a dep on Bar 1.2, and baz 2.3 takes a dep on Bar 1.4, both Bar contexts are tested with their respective libraries but forcing Bar to a particular global version can have problems and both versions of Bar are needed to have tested behavior. Examples mentioned and OP change the global behavior vs proper dependency management.

threePointFive 3y ago

I like Go's approach to this same idea. The standard database/sql library provides the standard API and the individual database drivers implement their own backend. You can use the URI connection string for your database (ie. postgres://...) though only after including the driver in your file's imports. There's even the idiomatic underscore prefix to the package import to note that you're only importing it for how it's presence affects another package. Unfortunately there's no way to sway which package you're affecting, but its still better than hidden changes.

didntcheck 3y ago
So basically JDBC? :) I think a similar approach is used with the crypto provider API and some others IIRC (in JDBC) you also used to have to do `Class.forName("name.of.it")` somewhere before trying to do any DB access, to ensure that the static initializers had actually run, but I don't believe it's necessary anymore (And then of course you have Spring Boot autoconfiguring which is another level of magic up, using automatic subclassing and proxy injection to add things like transaction management. And then you can get into proper classloader hackery)
- Groxx 3y ago
  Essentially, yes. And you need to register before use in Go too since reflection is far more limited in Go - often this is done at init time, so you just import the package that does the registration [somehow], but ultimately you just have to do it before you use something: https://pkg.go.dev/database/sql#Register https://pkg.go.dev/database/sql#Register The "data source name" string when connecting is... basically a JDBC connection string, and some adapters use exactly that iirc, but it's fundamentally an unstructured string that just serves the same purpose. Plugins can use anything they like, and style varies.

throwitaway222 3y ago

Disagree here. JDBC, ODBC, standards are great. There are old tools that can connect to generic databases because of said standards, you just need the right JDBC URL, or perhaps in Windows, ODBC. It's what makes interoperability work amazing things without having to recompile your code. "hidden monkeypatching" is essentially how all of Java works.

kstrauser 3y ago
Lacking much Java experience, I’ll accept this as true. If so, that’s another reason I’m uninterested in Java. Magic behavior changes based on the prefix of a string passed into a function doesn’t appeal to me at all. The Python equivalent for a database might look like (typed from memory): from psycopg import connect conn = connect(host, username, password, dbname) Where connect() returns a database adapter object with DB-API methods. When something breaks, I can see which DB code is involved. It’s right there in the import. My IDE doesn’t have to parse strings to know which code path subsequent method calls are following.
nightpool 3y ago
And it worked out super well for everybody in 2021 :)

valenterry 3y ago

> Explicit patching would be just fine Not fine. Just less horrible.

deleted 3y ago

[deleted]

mattpallissard 3y ago

> Hidden monkeypatching isn’t how we do things. The original authors of several codebases I've been saddled with over the years would disagree with you. This is Python and you a peasant. Yes most of the time the snake along the road is a snake, but occasionally it's a Basilisk waiting to destroy you with it's gaze. Knowing this, you the weary traveler, carry inconvenient tools such as mirrors to protect yourself and are well versed in superstition and arcane dark arts. You never let your guard down, because yes there is magic afoot.

kstrauser 3y ago
It's not like "Zen of Python" is an inviolable law, but things that break it are not, in my opinion, "Pythonic". The top 2 aphorisms are: "Beautiful is better than ugly." I think this is ugly. It's visually appealing, but my editor doesn't know that "csvbase" is a magic symbol that selects the code path to follow. That deep ugliness outweighs any skin-deep cleverness. "Explicit is better than implicit." That sums it up. This is implicit. `pip install foo` changing runtime behavior of packages that never import foo is as implicit as it gets. From a developer's and a security engineer's point of view, I loathe that code is running simply because it exists. It's possible to do these things. I contend that it's not Pythonic, and we should not be doing them.

everforward 3y ago

> This is Python. Hidden monkeypatching isn’t how we do things. It kind of is, though. This isn’t the first time I’ve seen this; off the top of my head, I believe the Stackdriver libraries do something similar but to the stdlib logging library. The fact that it’s possible is also a tacit endorsement of doing things this way. Many languages just don’t permit adding to or modifying a namespace. I would argue monkey-patching is a bad pattern in general, regardless of whether it’s manual or automatic. IMO, fsspec should have a private package-level variable with a list of these adapters, and expose a ‘fsspec.register_adapter(adapter)’ method to add things to it. I don’t see a need or use in patching here. It just seems fraught with issues that could be avoided with an adapter registry. Testing seems easier too; I can’t imagine the pain of unit testing a bunch of adapters if they each try to modify the fsspec package directly.

pantsforbirds 3y ago

Do you get upset when you see one of create_engine("postgresql://"), create_engine("mysql://"), or create_engine("sqlite://")? The author is just adding a new backend for a common protocol in the data world. Pandas, Polars, Dask, DuckDB, etc. all support this protocol and the type of people who want to access a dedicated CSV data archive would probably much rather keep their current client API and just add a new connection URI string vs. adding an entire client API for just one data source (or dealing with making requests and passing the data into the dataframe).

kstrauser 3y ago
I didn't say it was uncommon, just that it's a bad idea. There's no need whatsoever for a separate client API. There could be a convention like: from csvbase import loader as csvloader df = pd.read_csv(csvloader("calpaterson/onion-vox-pops")) The user wouldn't have to know anything but what to import to fetch a certain thing, and it's explicit about what's coming from there. There's also less risk of mistyping the URL string ("oops, I just typed cvsbase and accidentally loaded a list of CVS drugstores"), and code completion can tell you that csvloader() fetches things through the csvbase module. Cons: - It takes 5 seconds longer to type, one time. Pros: - You can tell what the code does at a glance.
mekoka 3y ago
You're missing the point. It's not about the use of bespoke URIs. This is the debate on "convention over configuration" revisited. Conventions are cute and always look clever while you're reading documentation. But they're a pain during maintenance, as they force others who don't know them to first go read the doc, instead of simply inferring from the previously researched and clearly stated declarations how all the wires connect. When I see `pd.read_csv("csvbase://")` during debug, I wonder how pandas knows to speak to csvbase (as the article anticipates). Nothing is imported. Nothing is configured. Things just speak to one another. So, can I also call pd.read_csv("other_csv_server://") like this? When I replace pandas with koalas, will koalas.read_csv("csvbase://") also work? How the wires connect between pandas and csvbase is hidden. Unless you know that the two are obeying some implicit lower layer (the fsspec standard), this becomes a mystery. Mysteries are the last thing you want when debugging. I don't know which `create_engine()` function you're alluding to. The one I know and have used comes from SQLAlchemy. How it works has always been obvious. I've never seen any mention of fsspec. I looked at its code and it's predictably just a convenient syntax to specify connection information in a single string. The string is simply parsed to extract connection attributes, which are then relayed to the lower DBAPI. There's no mystery involved.

calpaterson 3y ago

I'm sorry you're not a fan :) I do take your point - to an extent. I think the subject you're touching on is basically that of configuration. Configuration is what allows you to change the behaviour of code without making a code change. Configuration is of course very really powerful for good and evil. Someone below mentions ODBC which, yes, 100% is a great example. But the one that sticks in my mind is resolv.conf. I would contend (:)) that csvbase-client is not doing "fiddly magic at package installation time" but supplying configuration for a url scheme. In much the same way that installing boto3 does for using the `s3://` url scheme. "Magic" in my book would be monkeypatching the methods of other libs. I would hate to write `csvbase_patch` and have it execute on the import of a specific module. Like you, I prefer to live in a low-magic fantasy world.

kstrauser 3y ago
To be super clear, I'm not a fan of this specific thing. The rest of csvbase may be the best thing ever! This kind of configuration feels very magical in that it's completely behind the scenes. There's nothing in code I can look at that says "this URL should be handed to this adapter". There's no env var that gets slurped in. No .env file that's parsed. There's nothing checked into git that tells me how that URL scheme could possibly work, except the existence of a package in pyproject.toml -- one that's not even imported, just present. That makes it stateful in a bad way. If `poetry install` (or `pip install -r requirements.txt` or whatever) didn't complete, the module using it will still load and run up until the point that it crashes because that URL doesn't load. If I ran an automated process to prune unused modules from poetry/pip, suddenly behavior changes. I don't like that idea one bit.