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

migrate to pyproject #117

Merged
merged 2 commits into from
Sep 13, 2023
Merged

migrate to pyproject #117

merged 2 commits into from
Sep 13, 2023

Conversation

knaaptime
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Sep 11, 2023

Codecov Report

Merging #117 (fd0ba6c) into main (416bca9) will increase coverage by 4.0%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #117     +/-   ##
=======================================
+ Coverage   59.6%   63.5%   +4.0%     
=======================================
  Files         10      11      +1     
  Lines       1405    1404      -1     
  Branches     246       0    -246     
=======================================
+ Hits         837     892     +55     
+ Misses       518     512      -6     
+ Partials      50       0     -50     

see 9 files with indirect coverage changes

"Intended Audience :: Science/Research",
"Topic :: Scientific/Engineering :: GIS",
]
requires-python = ">=3.7"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
requires-python = ">=3.7"
requires-python = ">=3.8"

We should also drop 3.8, but that can be in another PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

i wasnt sure how to set this. Is that what we chose for all the other packages? I figure the minimum supported version isnt the same thing as the minimum required version, and this will probably run on python 3.6+ if i had to guess, so didnt want to force a more recent version, even if its technically unsupported

basically im worried that if we set this too high, it will cause unnecessary install failures, when we should allow people to install unsupported versions if they want

Copy link
Member

Choose a reason for hiding this comment

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

We should just follow SPEC0. Then we don't need to have these discussions.

[tool.ruff]
line-length = 88
select = ["E", "F", "W", "I", "UP", "N", "B", "A", "C4", "SIM", "ARG"]
target-version = "py39"
Copy link
Member

Choose a reason for hiding this comment

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

This should technically be py38, but seems to be safe to leave as is for now, since we will drop 3.8.

@martinfleis
Copy link
Member

build_docs.yml needs an additional step installing the package with pip install . to retrieve the version before make html can be called.

@jGaboardi
Copy link
Member

build_docs.yml needs an additional step installing the package with pip install . to retrieve the version before make html can be called.

xref: pysal/mapclassify@388c7e6

@knaaptime
Copy link
Member Author

build_docs.yml needs an additional step installing the package with pip install . to retrieve the version before make html can be called.

@martinfleis like that?

@martinfleis
Copy link
Member

Doesn't need to be editable but this should do the trick, yes.

@knaaptime
Copy link
Member Author

while we consider spec0 globally, should we consider this ready to go for now?

Copy link
Member

@martinfleis martinfleis left a comment

Choose a reason for hiding this comment

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

I think it is ready.

@jGaboardi jGaboardi merged commit 8fd02a6 into pysal:main Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants