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

Replace setup.py with pyproject.toml #242

Merged

Conversation

mcoughlin
Copy link
Member

This PR tries to replace setup.py with pyproject.toml.

Update authors
@sahiljhawar sahiljhawar merged commit 35aca1b into nuclear-multimessenger-astronomy:main Sep 19, 2023
@sahiljhawar
Copy link
Member

LGTM. Resolves #241

@sahiljhawar
Copy link
Member

@mcoughlin Sorry I didn't notice but all the entry points has been renamed. Should I change all of them?

@mcoughlin
Copy link
Member Author

@sahiljhawar I think by convention, they should be "-" rather than "_"

@sahiljhawar
Copy link
Member

yes, but this would mean that old workflow will be broken, those who are accustomed to do light_curve_analysis, etc

@mcoughlin
Copy link
Member Author

@sahiljhawar I think we should make an announcement on list.

@sahiljhawar
Copy link
Member

Okay! Docs needs to be updated as well.

@tylerbarna
Copy link
Collaborator

@mcoughlin @sahiljhawar once we update the docs, perhaps we should tag this as a new release? since this alters the way users interact with most of the functions

@tylerbarna
Copy link
Collaborator

It might also be worth (if possible) having some small period of deprecation where the old names are still callable to minimize compatibility issues with any users who updated without updating their existing codes that rely on nmma

@sahiljhawar
Copy link
Member

But if we give the upgrade window, people will still be using the old names. How often do we seriosuly care about numpy or scipy deprecations?👀

@mcoughlin
Copy link
Member Author

I think as long as we do a bump to 0.1.0 we can justify the change.

@tylerbarna
Copy link
Collaborator

I think as long as we do a bump to 0.1.0 we can justify the change.

that's a good point

@tylerbarna
Copy link
Collaborator

A bump to 0.1.0 may also be a good time to start doing release notes

@mcoughlin
Copy link
Member Author

@sahiljhawar Can you write some release notes and then try a 0.1.0 release? I just pushed changes to the docs.

@tylerbarna
Copy link
Collaborator

tylerbarna commented Sep 19, 2023

@mcoughlin @sahiljhawar while we're at it, we may as well remove the KNTimeshift deprecation warning if we're upgrading to 0.1.0

relevant commit: 4e30aad

@sahiljhawar
Copy link
Member

@tylerbarna Yes!

@tylerbarna
Copy link
Collaborator

@sahiljhawar just submitted #243 to address this

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.

3 participants