mhe / pynrrd

Simple pure-python module for reading and writing nrrd files.
https://pynrrd.readthedocs.io/
MIT License
116 stars 51 forks source link

Add fmt and lint checks #118

Closed bernardopericacho closed 2 years ago

bernardopericacho commented 2 years ago

Proposed changes

addisonElliott commented 2 years ago

@bernardopericacho After this PR, do you think we’re getting close to a v1 release or are there other major changes you had in mind?

bernardopericacho commented 2 years ago

@bernardopericacho After this PR, do you think we’re getting close to a v1 release or are there other major changes you had in mind?

I was thinking about pinging you (somehow) to talk about a couple of things:

addisonElliott commented 2 years ago

Sorry for the delay! This fell off my radar.

I personally don't think the API is too confusing right now as is. Plus, I think the examples are verbose & helpful enough that it resolves any potential confusion.

I think reading/writing directly to/from files is the majority use-case for this library. Could be wrong here. It saves a few steps of the user having to open the file handle. Also, it allows us to read in detached-header NRRD files when reading by opening the header file for them.

  • the other one was typing hints. What are your thoughts about it?

That sounds good to me. I'm a big fan of type hinting.

  • Also, looks like numpydoc dependency is only used for generating the documentation? (please correct me if I am wrong) it is needed as a package dependency?

Yep, that's correct. IIRC, it was necessary so ReadTheDocs would know to download that. I should be able to do this a different way now.

addisonElliott commented 2 years ago

Yep, that's correct. IIRC, it was necessary so ReadTheDocs would know to download that. I should be able to do this a different way now.

I did this in #120, so we're good to go there.