converting this repo to a python project

psteinb commented 8 months ago

As discussed in #8 here is a small restructuring of the repo to become a full python package. I'd like to add one more thing before this is ready for merge:

[x] add github actions integration of pytests

psteinb commented 8 months ago

@greydanus This PR is ready to go for my taste. There are a couple of things missing though, which I haven't tended to yet (I'd love getting feedback from you or the community at large first):

to run the notebooks (now in a subfolder), I'd need to make sure that the additional modules specific to them are available
I have kept the default pickle file where it is, this could be included in the package distribution later
once this is merged and tested, one should go ahead and start experimenting with pushing this to pypi (after successfully tagging)
in the best of all worlds, pushing the repo to pypi should be performed as a github action once a new release is tagged; see this guide for more details
there should be more documentation in the core API
the publication of this dataset should involve getting a DOI, a good start to achieve this would be to link the repo to zenodo

greydanus commented 8 months ago

@psteinb Thanks for getting this rolling! I'm at work right now, but will go over the changes line by line this evening. Hopefully we can get this in shortly.

greydanus commented 8 months ago

Note to say that I have not forgotten about this merge. It's on the todo list. I'm working on a paper submission right now and will merge this change post paper deadline to avoid breaking anything.

dkobak commented 7 months ago

Hi Peter. Looks like a great idea and an excellent PR.

Sam and I have been recently working on some updates to the notebooks, and I went ahead right now and added an additional notebook to the newly created notebooks folder. Our idea is eventually to put all notebooks that produce figures in the paper into that folder.

After that, I noticed that your PR also creates a notebooks folder, so I am not sure if there going to be any conflicts when Sam tries to merge it. Would you need to update the PR somehow? If so, apologies for that.

Another question: the entire notebooks folder may not need to be included into the project though. I guess if somebody does pip install mnist1d, they want to have access to the functions that generate the dataset. But they don't need the notebooks installed somewhere in ~/.local. How did you set it up?

psteinb commented 7 months ago

@dkobak this is great to hear! I am happy to adapt the PR whereever needed. the notebooks folder can be pulled up from the src. This is not a big issue for me. I can tackle this towards end of Feb.

On the same note, I agree that "we" should be careful NOT to push the notebooks to PyPI. This can be done in pyproject.toml to ignore all *.ipynb files for example when building the project. So I don't see any problems here. From my point of view, the beauty of mnist1d is that it is super lightweight. This needs to be retained.

I've set this PR to draft mode. I have some time to devote to this in about 2 weeks. So if there are major code changes with respect to the repo structure, please push/merge them until then. I'd then port my PR to whatever state the repo is in. I'll ping you guys whenever I am ready for merging. This should then allow people to do:

$ python -m pip install git+https://github.com/greydanus//mnist1d.git

Once this is done, I suggest to have a direct conversation who will push to PyPI and other related issues going forward.

psteinb commented 6 months ago

@dkobak @greydanus Sorry to reply so late. I included your changes in this PR. The package is ready to be pushed to PyPi. It would be great if you could review my changes, provide feedback and potentially merge.

Once this PR is merged, the next step would be to agree who gets credentials to manage the pypi distribution. I can continue to help with this. Just let me know. I'd also be interested to include this package in our teaching efforts. I think we should perhaps meet and discuss all of this. Happy to take this offline.

greydanus commented 6 months ago

Responding to comments on our MNIST-1D paper submission this weekend (feeling good about its fate). Will help get this merged after that is finished. Thanks again Peter - and sorry for the spottiness on my end. We'll get this in soon.

greydanus commented 4 months ago

Merged!

greydanus commented 4 months ago

@psteinb I just fixed a number of finer points (eg, switching git workflows over to running on this repository rather than your fork). Tests are passing now. Also, for the four notebooks in the notebooks/ repo, all run locally and all run on Colab as well.

Next goal is to bring the other notebooks (currently hosted on Drive as Colabs) into the notebooks directory, and debug in a similar manner. This goal is orthogonal to adding pip support, which I believe is complete and works quite well.

Thank you for this excellent pull request!

dkobak commented 4 months ago

Hi Sam and Peter -- do I understand correctly that while the package can now be installed via

pip install git+https://github.com/greydanus/mnist1d.git@master

it is not on available on PyPI and so can't be installed as pip install mnist1d? Why not? I think it should be straightforward to upload it there? Should we do it?

greydanus commented 4 months ago

We should do it. I haven't done this before. I'll look into it. @psteinb have you done this before?

psteinb commented 4 months ago

I am totally for uploading and sharing mnist1d on pypi. See also my comment on this topic previously. As far as I understand, someone has to sign up on pypi and manage the uploads. AFAIK, this can be automated via github actions.

greydanus commented 4 months ago

Great. I’ll sign up and manage, as I am already the repo owner.

On Thu, May 16, 2024 at 12:39 PM Peter Steinbach @.***> wrote:

I am totally for uploading and sharing mnist1d on pypi. See also my comment on this topic previously https://github.com/greydanus/mnist1d/pull/9#issuecomment-2015156324. As far as I understand, someone has to sign up on pypi and manage the uploads. AFAIK, this can be automated via github actions.

— Reply to this email directly, view it on GitHub https://github.com/greydanus/mnist1d/pull/9#issuecomment-2116048721, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEN5TDVCCN5GKBYXZDAWDILZCUDQBAVCNFSM6AAAAABB4NXHJ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJWGA2DQNZSGE . You are receiving this because you modified the open/close state.Message ID: @.***>

dkobak commented 4 months ago

@psteinb By the way, would it be okay with you if we add you to the acknowledgements "For help with preparing the Python package"? I hope you don't mind!

greydanus / mnist1d

converting this repo to a python project #9