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

Issue hotfix #198

Merged
merged 7 commits into from
Jul 15, 2024
Merged

Issue hotfix #198

merged 7 commits into from
Jul 15, 2024

Conversation

rcervinoucm
Copy link
Collaborator

Issue solving #195 and #196

Updating for Issue #196
Update for Issue #196
Creation of a first version for Issue #195
@rcervinoucm rcervinoucm requested a review from TjarkMiener July 15, 2024 08:16
pyproject.toml Outdated Show resolved Hide resolved
Copy link
Member

@TjarkMiener TjarkMiener left a comment

Choose a reason for hiding this comment

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

Thanks @rcervinoucm

@rcervinoucm rcervinoucm merged commit 6d2bed3 into master Jul 15, 2024
4 checks passed
@TjarkMiener TjarkMiener deleted the issueHotfix branch July 15, 2024 08:43
@@ -22,9 +25,11 @@ def getVersionFromFile():
url='https://github.com/ctlearn-project/ctlearn',
license='BSD-3-Clause',
packages=['ctlearn'],
install_requires=getRequirements(),
Copy link
Contributor

Choose a reason for hiding this comment

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

You should not duplicate information between pyproject.toml and setup.py.

This can lead to confusion and errors.

After you introduced pyproject.toml with all the configurations you provided, you should just remove the setup.py.

@@ -9,6 +9,9 @@ def getVersionFromFile():

here = path.abspath(path.dirname(__file__))

def getRequirements():
return open("docs/requirements.txt").readlines()
Copy link
Contributor

Choose a reason for hiding this comment

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

Requirements for building the docs should go into the project.optional-dependencies section of pyproject.toml

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

@rcervinoucm rcervinoucm Jul 15, 2024

Choose a reason for hiding this comment

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

We have the dependencies in the setup.py and in the dependencies section in the pyproject.toml file, they should also be included in the optional-dependencies? In our case they are not optional

dependencies = [
"dl1_data_handler>=0.12.0",
"astropy",
"matplotlib",
"numpy",
"pandas",
"pip",
"pyyaml",
"scikit-learn",
"ctaplot",
"numba>=0.56.2,<0.57",
"tensorflow>=2.15,<2.16",
"pydot",
"pyirf",
]

Copy link
Contributor

Choose a reason for hiding this comment

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

I misunderstood then. I thought docs/requirements.txt are additional requirements that are needed to build the documentation (e.g. sphinx)

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