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

mesh_doctor installation via pip #18

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

CusiniM
Copy link
Collaborator

@CusiniM CusiniM commented Apr 12, 2024

I noticed that the installation of mesh_doctor was not working.

This PR allows to install mesh_doctor like all other geos python packages.

@CusiniM CusiniM self-assigned this Apr 12, 2024
@CusiniM CusiniM requested review from TotoGaz and cssherman April 12, 2024 18:16
@CusiniM
Copy link
Collaborator Author

CusiniM commented Apr 12, 2024

I suspect that something will have to be fixed for the testing to work.

@CusiniM
Copy link
Collaborator Author

CusiniM commented Apr 12, 2024

I suspect that something will have to be fixed for the testing to work.

never mind. Looks like we are not running pytest anyways. I just need to format the files.

@untereiner
Copy link

Shouldn’t #13 come first ?

@CusiniM
Copy link
Collaborator Author

CusiniM commented Apr 13, 2024

Shouldn’t #13 come first ?

Rather than come first, I assume that #13 already solves the issue? If that's the case, we can also discard this PR. It really depends on what the timeline is. Coz, currently, the installation does not work which was annoying for me coz I needed to use it.

If the other PR fixes it and it will be merged quickly we can discard this PR once that one is merged. If there is still work to do I would prefer merging this patch to fix the issue quickly.

@untereiner
Copy link

I don't know the timeline. I was hoping #13 will be merged sooner. I suppose it has to wait #14.
Not sure #13 fixes your issue by the way. I'd address the packaging in a separate PR.
Maybe merge your I will handle the numerous conflicts afterwards.

@CusiniM
Copy link
Collaborator Author

CusiniM commented Apr 17, 2024

I don't know the timeline. I was hoping #13 will be merged sooner. I suppose it has to wait #14. Not sure #13 fixes your issue by the way. I'd address the packaging in a separate PR. Maybe merge your I will handle the numerous conflicts afterwards.

the changes in this PR are minimal. All I did was just adding an init file and fixing the steup.cfg. The toml file should address the issue of having the package well defined. I can simply try to go in your PR and see if the installtion works and fix it if doesn't. this way you won't have to deal with annoying conflicts due to name changes.

@TotoGaz
Copy link
Contributor

TotoGaz commented Apr 18, 2024

Hello guys, it looks to me that #13 handles all the deployments issues of all our python tools at once and that it has been waiting for some time.
That would make sense to be that #13 gets merged since it looks wider and was first.
@cssherman What's your take on this, it's your repo now 🤣

@cssherman
Copy link
Collaborator

Hello guys, it looks to me that #13 handles all the deployments issues of all our python tools at once and that it has been waiting for some time. That would make sense to be that #13 gets merged since it looks wider and was first. @cssherman What's your take on this, it's your repo now 🤣

We still need to iron out some issues with #13 in the main GEOS repo, and I hope to be able to look at it next week. If that works, then it could certainly take priority.

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

Successfully merging this pull request may close these issues.

4 participants