🔒 General feedback on my command-line tool?

[proper wave]

Hi there!
I just released my first Python package. It's a command-line tool that reduces the size of GeoJSON files.
I was hoping to get some feedback (good or bad) on any aspect of the project:
https://github.com/ben-n93/geojson-shave

Thanks!

[hearty river]

@proper wave

Python help channel opened

Remember to:

• Ask your Python question, not if you can ask or if there's an expert who can help.
• Show a code sample as text (rather than a screenshot) and the error message, if you've got one.
• Explain what you expect to happen and what actually happens.

:warning: Do not pip install anything that isn't related to your question, especially if asked to over DMs.

Closes after a period of inactivity, or when you send !close.

[digital solstice]

what kind of feedback are you looking for?

[proper wave]

I guess I'm most interested in the code quality, if that clears it up. Is the design well thought out? Any anti patterns? Or just anything obviously bad?

[digital solstice]

don’t quote me on this, but i think you need a pyproject.toml file specifying the build system

unless setup.cfg is a proper thing for doing that, but im not sure

[proper wave]

I believe that pyproject.toml is the preferred/newer way of doing it but setup.cfg still worked for me.
https://stackoverflow.com/questions/39484863/whats-the-difference-between-setup-py-and-setup-cfg-in-python-projects

[digital solstice]

does pip complain about deprecation warnings when you build your project?

[proper wave]

I honestly can't recall.. I don't think so. But for my next project I definitely plan on using .toml
I think I just wanted to use setup.cfg because I see a lot of packages use it and I was curious lol

[digital solstice]

i see you raise a SystemError, that’s really only supposed to be used from pythons C API (as far as i know, at least)

i suggest raising a RuntimeError instead

[proper wave]

Ah okay cool. Great to know.

[proper wave]

Do you know if there are any resources as to what exception to use when? I mean I'd know to raise a ValueError if an int is passed instead of a string but for other scenarios

[digital solstice]

you would raise a TypeError for that actually

ValueError is for when something is the wrong value, not the wrong type

[proper wave]

Oh gosh. There you go.

I did know that...

Haha

Thanks 😉

[digital solstice]

i tend to use RuntimeError for everything else

[proper wave]

Okay that's really good to know. I'll update the code and push it out on the next package release.

[digital solstice]

if you see a SystemError without using the C API, it means there’s probably something very wrong

[proper wave]

Ah okay right right

[digital solstice]

this one is complete suggestion and not a problem with your code, but if you ever feel that your json parsing is too slow, consider using ujson or orjson

i tend to use those over the built in json for most projects

[proper wave]

Oh sweet. I'll keep that in mind.

Thank you

How long have you been programming for?

You know so much haa

[digital solstice]

nearly 7 years now

there’s people who are much smarter than me here

[proper wave]

I really appreciate you taking the time to look at my project. I greatly appreciate it.
Any other general comments? You can be brutally honest!

[digital solstice]

im a bit confused on what this is doing

except TypeError:  # Feature's "geometry" member has a null value.
                    progress_bar()
                    continue
                progress_bar()

won’t it just call progress_bar and go to the next iteration in both cases?

if you want to just ignore an exception, see contextlib.suppress

!d contextlib.suppress

[hearty river]

contextlib.suppress

contextlib.suppress(*exceptions)```
Return a context manager that suppresses any of the specified exceptions if they occur in the body of a `with` statement and then resumes execution with the first statement following the end of the `with` statement.

As with any other mechanism that completely suppresses exceptions, this context manager should be used only to cover very specific errors where silently continuing with program execution is known to be the right thing to do.

For example...

[proper wave]

Yeah you're completely right.
I wasn't aware of contextlib.suppress, thanks!

[digital solstice]

this one’s a total nitpick: you reference some dictionary keys a lot (such as output_geojson["feature"]), it could be a good idea to store that in a variable

there’s nothing at all wrong with what youre doing though, if you want to keep it that way go for it

[proper wave]

Valid comment.

Thanks

Good idea

[digital solstice]

the rest looks pretty good to me though, nice project

if i were to really nitpick something, i would say add some information about each parameter on the docstrings (or add type hints)

[proper wave]

Haha I was definitely being a little lazy... but good point. I'll probably regret it 6 months+ when I go back to this and need a remidner on something

Thanks so much man. I really appreciate it. I know it's not much but I contributed some money to you on Github. Keep up the good work!

[digital solstice]

oh wow thanks, normally you don’t get paid for this sort of stuff

[proper wave]

no problem. enjoy your day/evening 🙂

!close

[hearty river]

Python help channel closed

This help channel has been closed and it's no longer possible to send messages here. If your question wasn't answered, feel free to create a new post in #1035199133436354600. To maximize your chances of getting a response, check out this guide on asking good questions.