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

Set up versions with last digit a pyerfa fix number. #30

Merged
merged 2 commits into from
May 31, 2020

Conversation

mhvk
Copy link
Contributor

@mhvk mhvk commented May 10, 2020

My attempt at the four-digit version number. Still not sure I actually like it all that much, since the way things are set up, the python wrappers are fairly erfa-version agnostic.

Also note the hack in the tests to work around the fact that in our self-compilation we get the wrong erfa version number (see liberfa/erfa#62)

Copy link
Member

@avalentino avalentino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looks very good to me, anyway after the merge of #29 there is now a conflict.
Of course, once we merge this PR the issue #7 should be considered closed.
Also it would be nice to have a note somewhere (maybe in RELEASING.rst) about the versioning scheme.

setup.py Outdated

ERFA_SRC = os.path.abspath(os.path.join(ERFAPKGDIR, 'liberfa', 'erfa', 'src'))
LIBERFADIR = os.path.abspath(os.path.join(ERFAPKGDIR, 'liberfa', 'erfa'))
ERFA_SRC = os.path.abspath(os.path.join(LIBERFADIR, 'src'))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consider to adjust the above lines to avoid absolute paths (see also #29).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also:

ERFA_SRC = os.path.abspath(os.path.join(LIBERFADIR, 'src'))

should become

ERFA_SRC = os.path.join(LIBERFADIR, 'src')

Also the Extension object seems to injects paths into the egg-info/SOURCES.txt

@mhvk
Copy link
Contributor Author

mhvk commented May 11, 2020

I'm still not sure about the version number, but will continue that discussion in #7.

@avalentino avalentino mentioned this pull request May 16, 2020
@mhvk mhvk mentioned this pull request May 31, 2020
10 tasks


# https://mail.python.org/pipermail/distutils-sig/2007-September/008253.html
class NumpyExtension(setuptools.Extension):
"""Extension type that adds the NumPy include directory to include_dirs."""

def __init__(self, *args, **kwargs):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this as we do not use cython so it seemed meaningless.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1



# https://mail.python.org/pipermail/distutils-sig/2007-September/008253.html
class NumpyExtension(setuptools.Extension):
"""Extension type that adds the NumPy include directory to include_dirs."""

def __init__(self, *args, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@mhvk
Copy link
Contributor Author

mhvk commented May 31, 2020

OK, let's get this in. With #33 I think we'll be ready for a release!

@mhvk mhvk merged commit c47a204 into liberfa:master May 31, 2020
@mhvk mhvk deleted the pyerfa-version branch May 31, 2020 19:11
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.

2 participants