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

Python script to get an overview of changes with PyGithub #4277

Merged

Conversation

olexandr-konovalov
Copy link
Member

@olexandr-konovalov olexandr-konovalov commented Feb 19, 2021

See #4257 for the discussion.

@olexandr-konovalov olexandr-konovalov added the do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) label Feb 19, 2021
@olexandr-konovalov olexandr-konovalov force-pushed the release-notes-pygithub branch 3 times, most recently from 0a52a5f to 8b85e9c Compare February 19, 2021 15:43
@olexandr-konovalov
Copy link
Member Author

I will rebase this PR and squash some commits - @danielrademacher is this fine with you? You will have to reset your clone after that, if you will need to push new commits.

removelist = []
for k in prs:
#if not "release notes: use title" in item[2]:
if not "release notes: added" in prs[k]["labels"]:
Copy link
Contributor

Choose a reason for hiding this comment

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

For the next release notes we have to use "release notes: use title" instead of "release notes: added" at this point. At the moment we use "release notes: added" to see whether the script works with the current release notes.

@danielrademacher
Copy link
Contributor

Yes, of course. Thanks for telling me :)
And I really like the changes. The output looks even more amazing and storing the data locally is also an amazing new feature.

# TODO: for now this will do, use Sergio's code later
with open("/Users/alexk/.github_shell_token", "r") as f:
accessToken=f.read().replace("\n", "")
g=Github(accessToken)
Copy link
Contributor

@danielrademacher danielrademacher Feb 20, 2021

Choose a reason for hiding this comment

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

One last comment for anybody using this script:
You have two options to connect with Github:

  1. Use your username and password
  2. Use an access token (can be generated in settings -> Developer Settings -> Personal access token)

In any case, use the second variant with the token. If you otherwise push your version of the script, the password is in plaintext on Github.

@fingolfin
Copy link
Member

Does this already take backport labels into account / support patch releases? AS far as I could see, only major release (4.12.0) are supported but not minor/patch (4.11.1)

@olexandr-konovalov
Copy link
Member Author

olexandr-konovalov commented Feb 20, 2021

@fingolfin not yet. For that purpose, should I simply impose an additional condition for the appropriate backport label?

@fingolfin
Copy link
Member

Yes I think we simply filter based on whether the backport-to-4.11-DONE label is set or not. Also, anything with backport-to-4.11 but not backport-to-4.11-DONE should be inspected and possibly backported ASAP (or the backport label be dropped)

@olexandr-konovalov olexandr-konovalov force-pushed the release-notes-pygithub branch 3 times, most recently from dc1439c to a4f33f1 Compare February 23, 2021 22:35
@olexandr-konovalov
Copy link
Member Author

@fingolfin Ok - tried filtering those which are backported, you can see the result in #4257. Will add the logic for those labeled but not yet backported and push tomorrow. Also, rebased this PR and squashed some fixups.

dev/release-notes.py Outdated Show resolved Hide resolved
@@ -0,0 +1,211 @@
#!/usr/bin/env python3
#
# Usage: ./release-notes.py YYYY-MM-DD
Copy link
Member

Choose a reason for hiding this comment

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

This comment is redundant, and also not super helpful. Somewhere it should be stated what the YYYY-MM-DD is used for (either here in the comment, or in the usage() function, but not both).

And I guess you also need an optional argument to indicate whether this is for a minor or major release. There are many ways I can think of how that would look; but probably the easiest to implement is just allow passing in the name of an additional label to be used in queries. If one wants to be a bit more fancy, one can just take a version like 4.11 and then derive the backport-4.11-DONE label name from that. And of course even the 4.11 could be derived from the version of the GAP in the current directory, so then would just have to specify a flag --backports or --minor-release or so

dev/release-notes.py Outdated Show resolved Hide resolved
dev/release-notes.py Outdated Show resolved Hide resolved
dev/release-notes.py Outdated Show resolved Hide resolved
dev/release-notes.py Outdated Show resolved Hide resolved
dev/release-notes.py Outdated Show resolved Hide resolved
dev/release-notes.py Outdated Show resolved Hide resolved
for k in prs:
# The format of an entry of list is: ["title of PR", "Link" (Alternative the PR number can be used), [ list of labels ] ]
if "release notes: highlight" in prs[k]["labels"]:
# TODO: writing up these details should be a function
Copy link
Member

Choose a reason for hiding this comment

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

Indeed...

Copy link
Member Author

Choose a reason for hiding this comment

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

@fingolfin perhaps no need in a function, after it reduced to two lines...

dev/release-notes.py Outdated Show resolved Hide resolved
@olexandr-konovalov
Copy link
Member Author

@fingolfin what would the best way to get local copy of changes? Your changes went to the PR branch in the main repo, and @danielrademacher's pushed to a branch in my fork.

@olexandr-konovalov
Copy link
Member Author

Ah, that's not true - I allowed edits, and all changes are now in my fork. Then I know what to do.

dev/release-notes.py Outdated Show resolved Hide resolved
@fingolfin
Copy link
Member

This script could also generate a big part of the package update news, if we start accompanying the package distribution tarballs with gzipped JSON files containing the metadata, as generated by that script in dev/releases in the GAP repo

@ThomasBreuer
Copy link
Contributor

Just some feedback about the script:
In order to run it, first I had to install pygithub (seems to be nonstandard), perhaps a hint about this would be helpful.
Then I had to edit the script such that my own access token was used.
Afterwards, the script ran successfully.
However, it ran for more than half an hour. I understand that there is apparently no way to filter the pull request in advance by the date of the last change (very strange), but this means that one cannot use the script for "quickly getting an overview" of the changes since the previous release.

Another point:
I think it would be useful if the Json format file generated by the script (the file that lists the pull requests related to a GAP release) would become part of the distributed archive; more precisely, all these files for the releases (from now on) should be included. The file CHANGES.md can then be regarded as just one viewpoint of these data, for the impatient user.

Alexander Konovalov added 4 commits March 23, 2021 19:43
We need to use both old "release notes: added" label and
the newly introduced in "release notes: use title" label
since both label may appear in GAP 4.12.0 changes overview.
This makes unnecessary its special treatment early in the script.
Also improved comments and instructions for the unsorted PRs.
@olexandr-konovalov olexandr-konovalov changed the title [WIP] Python script to get an overview of changes with PyGithub Python script to get an overview of changes with PyGithub Mar 23, 2021
@olexandr-konovalov olexandr-konovalov removed the do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) label Mar 23, 2021
@olexandr-konovalov
Copy link
Member Author

This seems quite usable - maybe merge it and continue improvements in next PRs? I compared the output it would produce for 4.11.1 and a couple of discrepancies are all explainable (e.g. something that was already in GAP 4.11.0 and should have been reported before, or PR merged before GAP 4.11.0, but backported after.

Also, below is what it produces today for the major release.


Release Notes

New features and major changes

  • #3925 Added the missing perfect groups of order up to a million

Fixed bugs that could lead to incorrect results

  • #3695 Fix issue with test outputs which do not end with a newline when using the rewriteToFile option of Test
  • #4035 normalize package name to all-lowercase in DirectoriesPackagePrograms
  • #4206 Fixed DirectoriesPackageLibrary, DirectoriesPackagePrograms when they are called during package loading.
  • #4317 fixed Cite(); it had used the number of the day, not the year

Fixed bugs that could lead to crashes

Fixed bugs that could lead to break loops

  • #3693 Improve ViewString so Strings are correctly escaped
  • #3747 Fix unexpected error when testing permutation groups for conjugacy
  • #3903 Bug Fix: 'Cycles(( ),[ ])' now correctly returns '[ ]'
  • #4037 normalize package name to all-lowercase in IsPackageLoaded and GAPInfo.PackagesInfo
  • #4108 Fix some unexpected error when inverting or computing random elements of algebraic extensions of finite fields of size > 256
  • #4146 Fixed Cite w.r.t. the encoding used; let BibEntry always use encoding "UTF-8".
  • #4239 Added missing comparison w.r.t. equality of something and an object with memory (IsObjWithMemory).
  • #4245 Free (associative or Lie) algebras: admit non-fields as left acting domains, fixed the zero-dimensional case.

Other fixed bugs

  • #3761 Fix bugs in HPC-GAP serialization code and improve its tests
  • #3931 Do not ignore #@ comments after empty line at start of test file
  • #4219 Performance additions to generic 2-cohomology and Automorphism group/Isomorphism test

Improved and extended functionality

  • #3628 Add Info statement when assigning already assigned Attributes
  • #3643 attempt to make vector and matrix objects official
  • #3668 support several package instances with same version
  • #3713 Add tracing and counting of built-in operations
  • #3740 Speed up printing of large permutations
  • #3858 Add DeclareGlobalName to get rid of more uses of InstallValue
  • #3861 ENHANCE: Extended range of simple groups
  • #3862 Rename QUIT_GAP, FORCE_QUIT_GAP and GAP_EXIT_CODE to QuitGap, ForceQuitGap and GapExitCode (the old names remain available as synonyms)
  • #3901 Allow function-call syntax for general mappings.
  • #3911 PageSource now gives a hint, that ApplicableMethod can be used to find the methods installed for an operation
  • #3914 Added the attribute 'InnerAutomorphismGroup' for groups
  • #3920 Admit 'CentralIdempotentsOfAlgebra' for algebras without 'One'
  • #3933 Add libtool versioning to libgap shared library
  • #3986 Add some missing methods for machine floats (Sinh, Cosh, Tanh, Asinh, Acosh, Atanh, CubeRoot, Erf, Gamma)
  • #3992 Introduce a Pluralize function for strings
  • #4000 NormalClosure: accept list of normal generators as second argument
  • #4074 Add IteratorOfPartitionsSet an iterator for all unordered partitions of a set into pairwise disjoint nonempty sets
  • #4080 Allow registering global handlers for GAP_TRY/GAP_CATCH
  • #4099 Passing a .tst file as argument to gap will now invoke Test on it, instead of trying to Read it (which will fail)
  • #4105 The function OrderMod admits an optional third argument that is a multiple of the order one wants to compute
  • #4111 In FreeGroup, support the option generatorNames to prescribe the names of the generators.
  • #4145 Improved some errors messages for various set operations (AddSet, UniteSet, ...).
  • #4168 Improved the documentation concerning GASMAN, added CollectGarbage.
  • #4186 Improved the performance of CoeffientsQadic for long results.
  • #4201 Support an optional transformFunction in Test(), similar to compareFunction.
  • #4207 Added the group constructors PGammaL, PSigmaL, and their methods for permutation groups.
  • #4296 Allow function-call syntax for evaluating univariate polynomials

New features

  • #4104 Add TaylorSeriesRationalFunction and improve Derivative for univariate rational functions

Performance improvements

Improvements to the interface which allows 3rd party code to link GAP as a library

  • #4215 libgap API: Expose a minimal interface to the garbage collector, via GAP_MarkBag, GAP_CollectBags

Improvements in the support for using the Julia garbage collector

Changed documentation

Packages

Other changes

  • #3664 Tweak Rees matrix semigroups code
  • #3694 Deprecate undocumented extra arguments for DeclareGlobalFunction and InstallGlobalFunction
  • #3702 Make TmpNameAllArchs obsolete by enhancing TmpName
  • #3721 Add optimised SmallestMovedPoint implementation
  • #3759 Fix an issue with error reporting in parallel threads
  • #3792 Use Info not Print in Decomposition
  • #3796 Remove -B option for overriding the architecture
  • #3822 Make the assertion level (which controls Assert statements) thread local when using HPC-GAP
  • #3912 Update of GMP from version 6.1.2 (December 2016) to version 6.2.0 (January 2020)
  • #4023 Improve float literal parsing, fix some inconsistencies in it
  • #4026 For the buildsystem now only .d files that exist are included
  • #4126 Added a command line option --version.
  • #4128 Added OutputGzipFile, in order to create a gzip compressed file
  • #4132 kernel: remove output stream stack
  • #4142 Extend BindConstant, MakeConstantGlobal to all object types.
  • #4232 Iterators for lists: fixed IsDoneIterator, improved the performance

Copy link
Member

@wilfwilson wilfwilson left a comment

Choose a reason for hiding this comment

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

I'd be happy for this to be merged, with subsequent work being put into separate PRs.

It's a shame that the GitHub API for listing PRs doesn't offer as much filtering power as does the GitHub website, which means that you have to run through all PRs to get the ones you want. e.g. there's no easy way to only get your hands on the PRs in a list like:

https://github.com/gap-system/gap/pulls?q=is%3Apr+-label%3A%22release+notes%3A+not+needed%22+-label%3A%22release+notes%3A+added%22+is%3Aclosed+merged%3A2018-09-10..2021-09-10+base%3Amaster+is%3Amerged+

That's the main problem with this script, but I also can't see an obvious way round it.

I have just run the script (with both minor and then major) on my computer and the output seemed reasonable. It also took about 25 minutes.

@wilfwilson
Copy link
Member

(One option for a quicker script would be to scrape the HTML webpage of one of those long queries to get at least some initial data, possibly even the whole list of PR numbers that we want to investigate further. It's obviously somewhat unsupported, however.)

@ThomasBreuer
Copy link
Contributor

Concerning @wilfwilson's idea to "scrape the HTML webpage of one of those long queries":
This had been implemented by @danielrademacher.
The problem with such an approach is that "parsing" the HTML code must be based on assumptions on its structure, and sooner or later these assumptions will turn out to be wrong.
Still it would perhaps be a useful alternative in situations where one is interested in a quick overview of the current status.

@olexandr-konovalov
Copy link
Member Author

I have investigated where API queries are spent. My understanding is that you first call get_pulls, which is "cheap" and then get_labels for selected PRs which is more expensive since it's an API call for each PR merged after the start date. That costs API call, and also costs time since the call is not instant.

We are now going back to the history until September 2019 for the master branch. Had we released more often in the meantime, that would be faster. Maybe in the future with more often major release cycle this will be faster again.

OTOH, if at some point the problem will be that we're running out of 5000 calls, one could implement rate limiter, e.g. sleep for 5 second between iterations.

Even with that, I will not mind running this for an hour. It's not like dreaded task of collating PRs manually in markdown and then converting it to GAPDoc, even though semiatomatically.

@olexandr-konovalov
Copy link
Member Author

@wilfwilson btw, both minor and major use the same cache - I think once you did one, you should be able to run the other in seconds.

@wilfwilson
Copy link
Member

wilfwilson commented Mar 25, 2021

@alex-konovalov wrote:

My understanding is that you first call get_pulls, which is "cheap" and then get_labels for selected PRs which is more expensive since it's an API call for each PR merged after the start date. That costs API call, and also costs time since the call is not instant.

This seems to be incorrect. Yes, the initial "call" to get_pulls is cheap, but I think that's because that call doesn't do anything; in particular, it doesn't do an API call. The API is only called when the result of get_pulls is enumerated; i.e. it gets that data on demand, via successive API calls.

In particular, it seems that getting the PRs via get_pulls is expensive.

To be clearer, you are correct that this is very cheap, and quick:

all_pulls = repo.get_pulls(state="closed", sort="created", direction="desc", base="master")

But that's because it's not doing anything. Even just choosing a PR is expensive and slow:

x = all_pulls[0]

takes a couple of seconds.

In particular, running the following subset of your script:

>>> all_pulls = repo.get_pulls(state="closed", sort="created", direction="desc", base="master")
>>> for pr in all_pulls:
...     print(pr.number, end=" ")
...     sys.stdout.flush()
...     if pr.merged:
...             if pr.closed_at > datetime.fromisoformat(history_start_date):
...                     pass
...

has so far over taken 8 minutes, and I have only got from PR 4337 to PR 3200. I estimate, therefore, that the vast majority of the 25 minutes is required just to get the merging date of each PR, and overall, getting the labels of the relevant PRs is a small fraction of the 25 minutes.

In particular, the time taken by this script is almost completely independent of the value of history_start_date.

@wilfwilson
Copy link
Member

wilfwilson commented Mar 25, 2021

Note that calling via the HTTP API is seemingly a lot quicker, and gives you all of the information you want without having to load things in separately. Consider the following query: (click it!)

https://api.github.com/repos/gap-system/gap/pulls?state=closed&sort=created&direction=desc&per_page=25

The maximum value of per_page is 100, but you could load multiple pages via &page=2, etc.

You could then load as many of these pages as you want in Python via the requests module, parse the json into a dictionary, and then you've got all the information you want, including labels.

I don't know what the 'official' way of interacting with the HTTP API is, but there surely is one, and I'm not sure what the limits are, but it seems like this could hopefully lead to a script that takes on the order of seconds or minutes rather than half an hour.

@wilfwilson
Copy link
Member

@wilfwilson btw, both minor and major use the same cache - I think once you did one, you should be able to run the other in seconds.

The cache worked successfully for me 🙂

@danielrademacher
Copy link
Contributor

danielrademacher commented Mar 25, 2021

Thats my python script from the GAP Days which works with HTML:

import requests
import urllib
import time
from bs4 import BeautifulSoup


print("Please note that executing this script generates/overwrites a file with the name \"releasenotes.md\" and \"remainingPR.md\". If you want to quit this script now, please press 'CTRL' + 'C'.")


# Output: A file but here we work with a list at the start
#f = open("test.txt", "a")
list = []
counter = 0

# Input: Link for which we want to generate the release notes and the number of pages
# You have to use the link with the page number in it
print("Welcome to the release notes script. Please insert the GitHub link for the current release notes. Please make sure that the page number is in the link (click on page 2 und copy the link):\n")
url = input()
#url = 'https://github.com/gap-system/gap/pulls?page=2&q=is%3Apr+-label%3A%22release+notes%3A+not+needed%22+is%3Aclosed+merged%3A2019-09-10..2033-12-31+base%3Amaster+is%3Amerged'
print("Insert the number of pages for the GitHub search: \n")
numberpages = input()
#numberpages = 4
urltemp = url.split("page=")
url1 = urltemp[0]
url2 = urltemp[1]
while url2[0] != '&':
    url2 = url2[1:]

for pagenumber in range(1, numberpages+1):
    url = url1 + "page=" + str(pagenumber) + url2
    print(url)
    response = requests.get(url)
    html = response.text
    parsed_html = BeautifulSoup(html,features="html.parser")
    issues = parsed_html.findAll("div", {"class": "Box-row Box-row--focus-gray p-0 mt-0 js-navigation-item js-issue-row"})
    # print(horoscope)

    for item in issues:
        title = item.findAll("a", {"class": "Link--primary v-align-middle no-underline h4 js-navigation-open markdown-title"})
        list2 = []
        for item2 in title:
            temp = item2.encode('utf-8').split("_link")
            temp = temp[1]
            temp = temp.replace("\">", "")
            temp = temp.replace("</a>", "")
            list2.append(temp)
            temp2 = item2.encode('utf-8').split("href=\"")
            temp2 = temp2[1]
            temp2 = temp2.split("id=\"")[0]
            temp2 = temp2.strip()
            temp2 = temp2.replace("\"", "")
            temp2 = "https://github.com" + temp2
            #print(temp2)
            list2.append(temp2)
            list.append(list2)
            #f.write("%s\n" % temp)
            #f.write("----------------------------------------------\n")
            
    for item in issues:
        labellist = item.findAll("span", {"class": "labels lh-default d-block d-md-inline"})
        list2 = []
        for item2 in labellist:
            label = item2.findAll("a", {"class": "d-inline-block IssueLabel"})
            for item3 in label:
                temp = item3.encode('utf-8').split("\">")
                temp = temp[1]
                temp = temp.replace("</a>", "")
                list2.append(temp)
                #f.write("%s\n" % temp)
                #f.write("----------------------------------------------\n")
        list[counter].append(list2)
        counter = counter + 1

#f.write("-------------------------------------------------\n")
#f.write("------------------------+++----------------------\n")
#f.write("-------------------------------------------------\n")
    
    
#print(list)
#print(len(list))


# f.write("-------------------------------------------------\n")
# f.write("------------------------+++----------------------\n")
# f.write("-------------------------------------------------\n")


#for item in issues:
#        f.write("%s\n" % item)
#        f.write("----------------------------------------------\n")
#f.write(horoscope.encode('utf-8').strip())
#f.close()




# Generate the output file
f = open("releasenotes.md", "a")
f2 = open("remainingPR.md", "a")
f3 = open("releasenotes.json", "a")
jsonlist = list[:]

prioritylist = ["kind: bug: wrong result", "kind: bug: crash", "kind: bug: unexpected error", "kind: bug", "kind: enhancement", "kind: new feature", "kind: performance", "topic:libgap", "topic: julia", "topic: documentation", "topic: packages"]

f.write("Release Notes \n\n\n\n")
f.write("Category " + "release notes: highlight" + "\n")
removelist = []
for item in list:
    if "release notes: highlight" in item[2]:
        f.write("- [#")
        temp = item[1]
        length = len(temp)
        issuenumber = temp[length-4:length]
        f.write(issuenumber)
        f.write("](")
        f.write(temp)
        f.write(") ")
        f.write(item[0])
        f.write("\n")
        removelist.append(item)
for item in removelist:
    list.remove(item)
f.write("\n\n\n")

removelist = []
for item in list:
    if "release notes: not needed" in item[2]:
        removelist.append(item)
for item in removelist:
    list.remove(item)
    jsonlist.remove(item)

        
f2.write("Category " + "release notes: to be added" + "\n")
removelist = []
for item in list:
    if "release notes: to be added" in item[2]:
        f2.write("- [#")
        temp = item[1]
        length = len(temp)
        issuenumber = temp[length-4:length]
        f2.write(issuenumber)
        f2.write("](")
        f2.write(temp)
        f2.write(") ")
        f2.write(item[0])
        f2.write("\n")
        removelist.append(item)
for item in removelist:
    list.remove(item)
f2.write("\n\n\n")


f2.write("Uncategorized PR" + "\n")
removelist = []
for item in list:
    #if not "release notes: use title" in item[2]:
    if not "release notes: added" in item[2]:
        f2.write("- [#")
        temp = item[1]
        length = len(temp)
        issuenumber = temp[length-4:length]
        f2.write(issuenumber)
        f2.write("](")
        f2.write(temp)
        f2.write(") ")
        f2.write(item[0])
        f2.write("\n")
        removelist.append(item)
for item in removelist:
    list.remove(item)
f2.close()


for priorityobject in prioritylist:
    f.write("Category " + priorityobject + "\n")
    removelist = []
    for item in list:
        if priorityobject in item[2]:
            f.write("- [#")
            temp = item[1]
            length = len(temp)
            issuenumber = temp[length-4:length]
            f.write(issuenumber)
            f.write("](")
            f.write(temp)
            f.write(") ")
            f.write(item[0])
            f.write("\n")
            removelist.append(item)
    for item in removelist:
        list.remove(item)
    f.write("\n\n\n")
f.close()

f3.write("[")
for item in jsonlist:
    f3.write("%s\n" % item)
f3.write("]")
f3.close

The output is not corrected and improved like in Alexs script and so it looks like this at the moment:

Release Notes 



Category release notes: highlight
- [#3925](https://github.com/gap-system/gap/pull/3925) Added the missing perfect groups of order up to a million



Category kind: bug: wrong result
- [#4206](https://github.com/gap-system/gap/pull/4206) Fixed <code>DirectoriesPackageLibrary</code>, <code>DirectoriesPackagePrograms</code> when they are called during package loading.
- [#4178](https://github.com/gap-system/gap/pull/4178) Fixed bugs in <code>RestrictedPerm</code> with second argument a range.
- [#4035](https://github.com/gap-system/gap/pull/4035) normalize package name to all-lowercase in <code>DirectoriesPackagePrograms</code>
- [#3847](https://github.com/gap-system/gap/pull/3847) Fix printing of certain words in free groups (if subexpressions occur as powers iteratedly then nonsense could be displayed, but the data internally was correct)
- [#3738](https://github.com/gap-system/gap/pull/3738) Fix bug in <code>CycleStructurePerm</code> for a single cycle of length 2^16 that caused wrong answers and memory corruption
- [#3733](https://github.com/gap-system/gap/pull/3733) Fix ConstituentsOfCharacter for Brauer character
- [#3695](https://github.com/gap-system/gap/pull/3695) Fix issue with test outputs which do not end with a newline when using the rewriteToFile option of Test
- [#3690](https://github.com/gap-system/gap/pull/3690) Fix bug <code>NullspaceModQ</code> that could lead to wrong results; also added support for arbitrary moduli, and renamed it to NullspaceModN
- [#3689](https://github.com/gap-system/gap/pull/3689) Fix bug in <code>BlistList</code> for two ranges that could lead to wrong results
- [#3662](https://github.com/gap-system/gap/pull/3662) Fix bug in <code>MaximalSubgroupClassReps</code> that could lead to a wrong result



Category kind: bug: crash
- [#4076](https://github.com/gap-system/gap/pull/4076) Fix an infinite loop in series refinement if large factors could not be refined, improve documentation of <code>IsAutomorphismGroup</code> and fix typo in option name of <code>IsomorphismSimplifiedFpGroup</code>
- [#3965](https://github.com/gap-system/gap/pull/3965) Fix potential garbage collector crashes on 64bit ARM systems
- [#3825](https://github.com/gap-system/gap/pull/3825) Handle out of memory errors better (variant of PR #3793)
- [#3808](https://github.com/gap-system/gap/pull/3808) kernel: fix a crash and weirdness related to 'for' loops
- [#3715](https://github.com/gap-system/gap/pull/3715) Allow HPC-GAP to run as a forkable server process.



Category kind: bug: unexpected error
- [#4245](https://github.com/gap-system/gap/pull/4245) Free (associative or Lie) algebras: admit non-fields as left acting domains, fixed the zero-dimensional case.
- [#4239](https://github.com/gap-system/gap/pull/4239) Added missing comparison w.r.t. equality of something and an object with memory (<code>IsObjWithMemory</code>).
- [#4146](https://github.com/gap-system/gap/pull/4146) Fixed <code>Cite</code> w.r.t. the encoding used; let <code>BibEntry</code> always use encoding "UTF-8".
- [#4108](https://github.com/gap-system/gap/pull/4108) Fix some unexpected error when inverting or computing random elements of algebraic extensions of finite fields of size &gt; 256
- [#4037](https://github.com/gap-system/gap/pull/4037) normalize package name to all-lowercase in <code>IsPackageLoaded</code> and <code>GAPInfo.PackagesInfo</code>
- [#3980](https://github.com/gap-system/gap/pull/3980) FIX: GCD for rational polynomials
- [#3903](https://github.com/gap-system/gap/pull/3903) Bug Fix: 'Cycles(( ),[ ])' now correctly returns '[ ]'
- [#3865](https://github.com/gap-system/gap/pull/3865) Fix the code setting up a subgroup data structure by the solvable radical method, which could lead to unexpected errors
- [#3763](https://github.com/gap-system/gap/pull/3763) Fix bug in IrrConlon leading to unexpected errors
- [#3747](https://github.com/gap-system/gap/pull/3747) Fix unexpected error when testing permutation groups for conjugacy
- [#3693](https://github.com/gap-system/gap/pull/3693) Improve ViewString so Strings are correctly escaped



Category kind: bug
- [#4219](https://github.com/gap-system/gap/pull/4219) Performance additions to generic 2-cohomology and Automorphism group/Isomorphism test
- [#4006](https://github.com/gap-system/gap/pull/4006) Fix gac to ensure that binaries it creates on Linux can load and run, even if a GAP package with a compiled kernel extension (such as IO) is present
- [#3963](https://github.com/gap-system/gap/pull/3963) Implement compression/decompression of filenames ending .gz
- [#3944](https://github.com/gap-system/gap/pull/3944) Bug Fix: The error checking in PartialPerm has been corrected such that invalid inputs (numbers &lt; 1) are detected
- [#3931](https://github.com/gap-system/gap/pull/3931) Do not ignore <code>#@</code> comments after empty line at start of test file
- [#3761](https://github.com/gap-system/gap/pull/3761) Fix bugs in HPC-GAP serialization code and improve its tests



Category kind: enhancement
- [#4207](https://github.com/gap-system/gap/pull/4207) Added the group constructors <code>PGammaL</code>, <code>PSigmaL</code>, and their methods for permutation groups.
- [#4201](https://github.com/gap-system/gap/pull/4201) Support an optional <code>transformFunction</code> in <code>Test()</code>, similar to <code>compareFunction</code>.
- [#4186](https://github.com/gap-system/gap/pull/4186) Improved the performance of <code>CoeffientsQadic</code> for long results.
- [#4168](https://github.com/gap-system/gap/pull/4168) Improved the documentation concerning GASMAN, added <code>CollectGarbage</code>.
- [#4145](https://github.com/gap-system/gap/pull/4145) Improved some errors messages for various set operations (<code>AddSet</code>, <code>UniteSet</code>, ...).
- [#4111](https://github.com/gap-system/gap/pull/4111) In <code>FreeGroup</code>, support the option <code>generatorNames</code> to prescribe the names of the generators.
- [#4105](https://github.com/gap-system/gap/pull/4105) The function <code>OrderMod</code> admits an optional third argument that is a multiple of the order one wants to compute
- [#4099](https://github.com/gap-system/gap/pull/4099) Passing a <code>.tst</code> file as argument to <code>gap</code> will now invoke <code>Test</code> on it, instead of trying to <code>Read</code> it (which will fail)
- [#4080](https://github.com/gap-system/gap/pull/4080) Allow registering global handlers for GAP_TRY/GAP_CATCH
- [#4074](https://github.com/gap-system/gap/pull/4074) Add <code>IteratorOfPartitionsSet</code> an iterator for all unordered partitions of a set into pairwise disjoint nonempty sets
- [#4000](https://github.com/gap-system/gap/pull/4000) NormalClosure: accept list of normal generators as second argument
- [#3992](https://github.com/gap-system/gap/pull/3992) Introduce a Pluralize function for strings
- [#3986](https://github.com/gap-system/gap/pull/3986) Add some missing methods for machine floats (Sinh, Cosh, Tanh, Asinh, Acosh, Atanh, CubeRoot, Erf, Gamma)
- [#3933](https://github.com/gap-system/gap/pull/3933) Add libtool versioning to libgap shared library
- [#3920](https://github.com/gap-system/gap/pull/3920) Admit 'CentralIdempotentsOfAlgebra' for algebras without 'One'
- [#3914](https://github.com/gap-system/gap/pull/3914) Added the attribute 'InnerAutomorphismGroup' for groups
- [#3911](https://github.com/gap-system/gap/pull/3911) PageSource now gives a hint, that ApplicableMethod can be used to find the methods installed for an operation
- [#3901](https://github.com/gap-system/gap/pull/3901) Allow function-call syntax for general mappings.
- [#3862](https://github.com/gap-system/gap/pull/3862) Rename QUIT_GAP, FORCE_QUIT_GAP and GAP_EXIT_CODE to QuitGap, ForceQuitGap and GapExitCode (the old names remain available as synonyms)
- [#3861](https://github.com/gap-system/gap/pull/3861) ENHANCE: Extended range of simple groups
- [#3858](https://github.com/gap-system/gap/pull/3858) Add DeclareGlobalName to get rid of more uses of <code>InstallValue</code>
- [#3840](https://github.com/gap-system/gap/pull/3840) Move MarkBag* funcs into GCs so they can inline MarkBag
- [#3790](https://github.com/gap-system/gap/pull/3790) Improvements to simple group generation/iteration
- [#3746](https://github.com/gap-system/gap/pull/3746) Allow package build systems to detect GAP version by inserting <code>GAP_VERSION</code> into <code>sysinfo.gap</code>
- [#3740](https://github.com/gap-system/gap/pull/3740) Speed up printing of large permutations
- [#3713](https://github.com/gap-system/gap/pull/3713) Add tracing and counting of built-in operations
- [#3668](https://github.com/gap-system/gap/pull/3668) support several package instances with same version
- [#3643](https://github.com/gap-system/gap/pull/3643) attempt to make vector and matrix objects official
- [#3628](https://github.com/gap-system/gap/pull/3628) Add Info statement when assigning already assigned Attributes
- [#3428](https://github.com/gap-system/gap/pull/3428) Fix GNU readline detection on OpenBSD, and make the configure test for it more robust



Category kind: new feature
- [#4104](https://github.com/gap-system/gap/pull/4104) Add TaylorSeriesRationalFunction and improve Derivative for univariate rational functions



Category kind: performance



Category topic:libgap



Category topic: julia
- [#4071](https://github.com/gap-system/gap/pull/4071) Make the Julia GC threadsafe when used from GAP.jl
- [#4058](https://github.com/gap-system/gap/pull/4058) Refine the logic for scanning Julia stacks
- [#4053](https://github.com/gap-system/gap/pull/4053) Fix the logic for scanning tasks in the Julia GC
- [#4042](https://github.com/gap-system/gap/pull/4042) Various updates for the Julia integration



Category topic: documentation



Category topic: packages
- [#4016](https://github.com/gap-system/gap/pull/4016) etc/Makefile.gappkg: various improvements
- [#3683](https://github.com/gap-system/gap/pull/3683) Add <code>etc/Makefile.gappkg</code>, for use in the build system of GAP package with kernel extensions

And

Category release notes: to be added
- [#4332](https://github.com/gap-system/gap/pull/4332) Use internal PLAIN_LIST_COPY to implement PlainListCopy, and document
- [#4327](https://github.com/gap-system/gap/pull/4327) Fix InvariantQuadraticForm for Omega(-1, 2*d, 2^n)
- [#4320](https://github.com/gap-system/gap/pull/4320) FIX: Outer automorphisms of certain Chevalley groups.
- [#4275](https://github.com/gap-system/gap/pull/4275) Enable SaveAndRestoreHistory by default
- [#4274](https://github.com/gap-system/gap/pull/4274) Enable color prompt by default
- [#4249](https://github.com/gap-system/gap/pull/4249) Add ARCH_IS_WSL, to detect Windows Subsystem for Linux
- [#4170](https://github.com/gap-system/gap/pull/4170) Use colors in output of <code>Test</code>
- [#3355](https://github.com/gap-system/gap/pull/3355) Allow packages to use ISO 8601 dates in their PackageInfo.g



Uncategorized PR
- [#4334](https://github.com/gap-system/gap/pull/4334) added the group constructors <code>PGO</code> and <code>PSO</code>
- [#4333](https://github.com/gap-system/gap/pull/4333) added <code>GO(1,q)</code>, <code>SO(1,q)</code>, <code>Omega(1,q)</code>, <code>Omega(-1,2,q)</code>
- [#4317](https://github.com/gap-system/gap/pull/4317) fixed <code>Cite()</code>; it had used the number of the day, not the year
- [#4296](https://github.com/gap-system/gap/pull/4296) Allow function-call syntax for evaluating univariate polynomials
- [#4295](https://github.com/gap-system/gap/pull/4295) Add OpenExternal, a function for opening files in the OS
- [#4273](https://github.com/gap-system/gap/pull/4273) Fix warning about checking unsigned &lt; 0 by making signed
- [#4267](https://github.com/gap-system/gap/pull/4267) buildsys: fix Makefile build order for GMP, zlib

I also had to make a small adjustment right now to get it working.

But the script is pretty fast.

@wilfwilson
Copy link
Member

This perhaps emphasises that it is a good goal to have the code for 'getting the data about merged PRs' and the code for 'parsing the data about merged PRs into release notes' be largely independent, which I think @ThomasBreuer might have suggested already.

Given the fast (and stable) HTTP API, I don't think it will be necessary to maintain an HTML scraper, but since you've written it, it's good to have it – thanks for sharing @danielrademacher.

@olexandr-konovalov
Copy link
Member Author

olexandr-konovalov commented Mar 25, 2021

Thanks for correction, @wilfwilson - you are right that repo.get_pulls is just a handle, it costs no calls. I've instrumented the code to better see where calls are spent. It confirms what I had initially in mind, maybe poorly worded - PRs that do not get to get_labels call use one 1 API call per iteration in for pr in all_pulls loop. get_labels costs another API call. So the number of API calls is 2x + y, where x is the number of PRs we need for the release overview, and y is the number of PRs we don't. Eventually, we will reach the point when 2x+y > 5000. Either PyGithub & GitHub API will improve by that time (e.g. by allowing to sort the output by merging date instead of creation date), or we will find some optimisation, or we will have to use sleep to limit the rate. One way is to use a rate limiter like GeoPy does.

@ssiccha
Copy link
Contributor

ssiccha commented Apr 1, 2021

Hi folks, amazing work you did there! :)

I noticed that some sections of the generated markdown are empty, like

  • Performance improvements
  • Improvements in the support for using the Julia garbage collector
  • Changed documentation
  • Packages

I think on first sight it's not clear whether these headings are hierarchical or not. Could we just add a None. to each section which didn't have any matching PRs?

@ssiccha
Copy link
Contributor

ssiccha commented Apr 1, 2021

I agree with @alex-konovalov. Let's get this merged in and further improve this in additional PRs.

Wrt what @wilfwilson wrote:

Note that calling via the HTTP API is seemingly a lot quicker, and gives you all of the information you want without having to load things in separately. Consider the following query: (click it!)

https://api.github.com/repos/gap-system/gap/pulls?state=closed&sort=created&direction=desc&per_page=25

I've used per_page=100 and checked the headers. Under Response Headers > link for me it shows the links for the next and the last page of the data:
<https://api.github.com/repositories/31270208/pulls?state=closed&sort=created&direction=desc&per_page=100&page=2>; rel="next", <https://api.github.com/repositories/31270208/pulls?state=closed&sort=created&direction=desc&per_page=100&page=30>; rel="last"

When you arrive at the last page of the query, the Response Headers > link is
<https://api.github.com/repositories/31270208/pulls?state=closed&sort=created&direction=desc&per_page=100&page=29>; rel="prev", <https://api.github.com/repositories/31270208/pulls?state=closed&sort=created&direction=desc&per_page=100&page=1>; rel="first"
So then there are no "next" and "last" fields anymore.

@wilfwilson wilfwilson merged commit dec4c6a into gap-system:master Apr 1, 2021
@wilfwilson
Copy link
Member

I've (squashed and) merged - giving a nice base for future work. Let's try not to forget the comments/suggestions/discussions in this thread, though! 🙂

@fingolfin fingolfin deleted the release-notes-pygithub branch April 1, 2021 22:32
@fingolfin
Copy link
Member

This is a looong discussion, though -- if people think there are points in here we should not forget, I recommend to put them into one or multiple issues (possibly with a link to here in case there's additional context to be found)

@fingolfin fingolfin added the release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes label Feb 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants