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

overhaul infrastructure #579

Merged
merged 11 commits into from
Sep 30, 2023
Merged

overhaul infrastructure #579

merged 11 commits into from
Sep 30, 2023

Conversation

knaaptime
Copy link
Member

No description provided.

requirements_plus_pip.txt Show resolved Hide resolved
requirements_plus_conda.txt Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated
Comment on lines 11 to 14
{ name = "Martin Fleischmann", email = "martin@martinfleischmann.net" },
{ name = "Eli Knaap", email = "ek@knaaptime.com" },
{ name = "Serge Rey", email = "sjsrey@gmail.com" },
{ name = "Levi Wolf", email = "levi.john.wolf@gmail.com" },
Copy link
Member

Choose a reason for hiding this comment

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

What is the selection logic here?

Copy link
Member Author

Choose a reason for hiding this comment

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

no idea. its who was in base. hoping reviewers have opinions

Copy link
Member

Choose a reason for hiding this comment

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

In geopandas, in "authors" is only Kelsey Jordahl who founded the project, no one else. Maybe that would be the solution here as well? So only @sjsrey? Just a suggestion.

Copy link
Member Author

Choose a reason for hiding this comment

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

most of the other packages have two, so i think its probably appropriate to have it be serge and levi. I'll change it to that unless they think otherwise

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I don't know the history here.

Copy link
Member Author

Choose a reason for hiding this comment

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

well i get the logic, but maybe its a question that should be revisited occasionally instead of solved once. Serge, Ran and I were once the authors of spopt, but now it makes more sense for the authors to be James and Xin. ...but also a lot of the code was ported over from region and originally written by juanca, serge, and aleks. So its tricky

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'd probably characterize it as brute force rather than elegance, but sometimes any solution is preferable to nothing :P. By using founders as a replacement for authors, we're just saying our answer is to not address the question. That's fine, but it's a particular position (that assigning authorship is too hard to handle).

to throw out another few analogies, Guido is the author of Python, but at this point im sure he's the author of almost nothing in stdlib. Serge and Luc will forever be the authors of PySAL, but i think of authorship among the subpackages as additive, so the 'authors' are the respective contribution trees

i also think of pysal a bit like an edited handbook on spatial analytical methods. Serge and Luc are the editors of the handbook, and they wrote several chapters of the first edition. Many editions are pretty similar, where the same authors update their chapters in light of new evidence and techniques. But the content of the collection changes a bit over time as the discipline evolves and some of the authors rotate out, so chapters are eventually written by others. The subpackages in pysal are like the chapters, and lib is maybe the introduction to the series.

i think theres also an argument that the four of us sat down, scoped out, wrote, and revised this draft of the package (the graph-based version anyway), which makes us the authors of this version of the chapter. Eventually somebody else will probably write a new draft, at which point they assume 'authorship' (but we will always be in the contributor tree (and picked up by the zotero citation, for whatever thats worth)

Copy link
Member

Choose a reason for hiding this comment

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

I think that sounds reasonable. I think it's your call at this point @knaaptime.

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'm the resident dev with a community planning degree, so its only my job to raise questions. Answers are for the community :P. But jokes aside i'd probably just leave it at serge and levi

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with any solution. Just didn't want to exclude someone if we decide to name authors based on contributions. FYI, pandas has just Pandas development team as authors.

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
knaaptime and others added 3 commits September 29, 2023 12:08
Co-authored-by: James Gaboardi <jgaboardi@gmail.com>
Co-authored-by: Martin Fleischmann <martin@martinfleischmann.net>
@martinfleis
Copy link
Member

We need to add pip install . to the GHA to ensure we have access to __version__. That will fix the CI failure.

@codecov
Copy link

codecov bot commented Sep 29, 2023

Codecov Report

Merging #579 (e885c09) into main (3576691) will increase coverage by 1.5%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #579     +/-   ##
=======================================
+ Coverage   82.4%   83.9%   +1.5%     
=======================================
  Files        129     128      -1     
  Lines      15358   15021    -337     
=======================================
- Hits       12655   12610     -45     
+ Misses      2703    2411    -292     

see 1 file with indirect coverage changes

Co-authored-by: James Gaboardi <jgaboardi@gmail.com>
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.

One minor nit. Otherwise looks good!

name: libpysal-codecov

steps:
- uses: actions/checkout@v2
Copy link
Member

Choose a reason for hiding this comment

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

I think that the latest is v4 now (am on phone now)

@jGaboardi
Copy link
Member

Are we good to merge for now then, or are we waiting for something else?

@martinfleis
Copy link
Member

Two approvals are in and authorship can be discussed in the dev call and changed later if needed. Lgtm

@jGaboardi jGaboardi merged commit 479ce33 into pysal:main Sep 30, 2023
10 checks passed
@jGaboardi jGaboardi mentioned this pull request Oct 21, 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