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

pathlib requirement #8

Closed
xylar opened this issue Mar 22, 2021 · 9 comments
Closed

pathlib requirement #8

xylar opened this issue Mar 22, 2021 · 9 comments
Labels
enhancement New feature or request

Comments

@xylar
Copy link
Contributor

xylar commented Mar 22, 2021

pathlib is listed as a requirement for all python versions, but it is part of the system in python >=3.4 (i.e. all currently supported version of python). Because of this dependency, pip check fails when jigsawpy is installed for python 3.x. I would request that it either be removed as a dependency in the next release or that it be constrained to python versions before 3.4.

https://docs.python.org/3/library/pathlib.html

@dengwirda
Copy link
Owner

Okay, no worries --- the suggestion is just to remove REQUIRES = [..., pathlib, ...] from setup.py?

If memory serves, I believe this used to be required for python 3.6. Is there a chance this may have been backported since? Either way, I'll remove for the next jigsaw-python release.

@xylar
Copy link
Contributor Author

xylar commented Mar 22, 2021

Okay, no worries --- the suggestion is just to remove REQUIRES = [..., pathlib, ...] from setup.py?

Yes, exactly.

If memory serves, I believe this used to be required for python 3.6. Is there a chance this may have been backported since?

I don't believe so. It appears from the link above that it was part of python 3.4 from the get-go. Libraries are not generally added to python releases as backports, they remain as external packages that can be added, much like pathlib for python < 3.4.

Either way, I'll remove for the next jigsaw-python release.

Excellend, thanks!

@dengwirda dengwirda added the enhancement New feature or request label Mar 22, 2021
@jreniel
Copy link

jreniel commented Mar 22, 2021 via email

@dengwirda
Copy link
Owner

@xylar would you mind double checking the latest 0.9.15.rc that's now in master when you have a moment --- I've removed pathlib (but added scipy) to the setup.py REQUIRED list.

@xylar
Copy link
Contributor Author

xylar commented Apr 20, 2021

@dengwirda, sure, I can test that. For future reference, the cleanest way to do such a test would be for you to create a 0.9.15.rc1 tag (or whatever you want to call it, but best to number the RCs) of jigsaw and the equivalent for jigsaw-python. If you do this, I can first create the dev build of jigsaw on conda-forge, then use it in a dev build of jigsaw-python. Folks installing conda packages will only get these if they specifically as for the -c conda-forge/label/jigsaw_dev channel. The same could be done from the current master for each branch, but there's no "official" release candidate then.

For now, though, I'm going to just test with master in both places and name the RCs as best I can.

@xylar
Copy link
Contributor Author

xylar commented Apr 20, 2021

I'm also not sure I understand your jigsaw-python versioning. There seems to never have been a 0.3.4 and there was no release for 0.3.5 but we're now on 0.3.6(rc.1)?

@xylar
Copy link
Contributor Author

xylar commented Apr 20, 2021

@dengwirda, I made test builds of jigsaw and jigsawpy on conda-forge as you'll likely have seen. I made a test environment for COMPASS and was able to build a QU240 mesh with it without any trouble. The resulting meshes are quite different from what was produced with the previous jigsaw and jigsawpy but perhaps that is to be expected particularly at coarse resolution.

Previous jigsaw and jigsawpy releases:
old_jigsaw

Current release candidates:
new_jigsaw

@dengwirda
Copy link
Owner

Great, thanks @xylar. There may still be a few things to finalise for jigsaw-0.9.15.x, but I wanted to also check that the changes to REQUIRED are working for you here?

  • In terms of the precise layout of meshes, this essentially comes down to exact floating-point tie-breaks (wrt. the order that jigsaw inserts vertices), so I don't think it's possible to maintain exact consistency across versions. For these kind of semi-structured meshes though, I do think we should consider flipping to tetris (the multi-level scheme) at some point (post E3SM-v2 ;)), and look to reduce the number of non-hexagons, etc.
  • There were a few requests for new jigsaw-python functionality in-between this release and 0.3.3, so the version number has been incremented along a few times as a result. I don't think the jigsaw-python version has much meaning on its own, and I am tempted to just assign the jigsaw-python and jigsaw-matlab bindings the same digits as the underlying jigsaw release at this point. Would you have any issue with this?
  • Thanks for the info re: the conda rc workflow. I'll have to think about how best to maneuver the subtree based workflow through this too, but will chat with you offline if I have any questions on this.

@xylar
Copy link
Contributor Author

xylar commented Apr 20, 2021

wanted to also check that the changes to REQUIRED are working for you here?

Yep, that's great!

In terms of the precise layout of meshes, this essentially comes down to exact floating-point tie-breaks (wrt. the order that jigsaw inserts vertices), so I don't think it's possible to maintain exact consistency across versions. For these kind of semi-structured meshes though, I do think we should consider flipping to tetris (the multi-level scheme) at some point (post E3SM-v2 ;)), and look to reduce the number of non-hexagons, etc.

That's all fine. tetris would be interesting but the current behavior is fine.

There were a few requests for new jigsaw-python functionality in-between this release and 0.3.3, so the version number has been incremented along a few times as a result. I don't think the jigsaw-python version has much meaning on its own, and I am tempted to just assign the jigsaw-python and jigsaw-matlab bindings the same digits as the underlying jigsaw release at this point. Would you have any issue with this?

I think that would make a lot of sense, since you like to keep them in sync.

Thanks for the info re: the conda rc workflow. I'll have to think about how best to maneuver the subtree based workflow through this too, but will chat with you offline if I have any questions on this.

It's not a big deal. I just means that the rc1, rc2, etc. that I'm building on conda-forge could actually mean something to people other than me (and maybe you).

@xylar xylar closed this as completed Jul 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants