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

DOC: fix the dev version label in the version dropdown #55163

Closed
wants to merge 10 commits into from
Closed

DOC: fix the dev version label in the version dropdown #55163

wants to merge 10 commits into from

Conversation

hkhojasteh
Copy link
Contributor

@hkhojasteh hkhojasteh commented Sep 16, 2023

Fixing the version in dev distributions

Fixing the version in dev distributions
@hkhojasteh hkhojasteh changed the title Update conf.py DOC: fix the dev version label in the version dropdown #55160 modification at conf.py Sep 16, 2023
@hkhojasteh
Copy link
Contributor Author

hkhojasteh commented Sep 16, 2023

@jorisvandenbossche done at in-person sprint session :)

@jorisvandenbossche jorisvandenbossche added the Sprints Sprint Pull Requests label Sep 16, 2023
@jorisvandenbossche jorisvandenbossche changed the title DOC: fix the dev version label in the version dropdown #55160 modification at conf.py DOC: fix the dev version label in the version dropdown Sep 17, 2023
@jorisvandenbossche jorisvandenbossche added this to the 2.2 milestone Sep 17, 2023
Copy link
Member

@lithomas1 lithomas1 left a comment

Choose a reason for hiding this comment

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

Hmmm.
I'm not sure this is the right fix, but I don't know if it's wrong either.

Our doc preview bot is also down, so in the meantime, can you send a screenshot, showing how the change reflects in the docs?

@lithomas1
Copy link
Member

IIUC, we have some existing code that is supposed to do the conversion here.

pandas/doc/source/conf.py

Lines 234 to 238 in c253708

switcher_version = version
if ".dev" in version:
switcher_version = "dev"
elif "rc" in version:
switcher_version = version.split("rc", maxsplit=1)[0] + " (rc)"

(the .dev part is the relevant line)

Do you know if that broke somehow?

@hkhojasteh
Copy link
Contributor Author

hkhojasteh commented Sep 19, 2023

IIUC, we have some existing code that is supposed to do the conversion here.

pandas/doc/source/conf.py

Lines 234 to 238 in c253708

switcher_version = version
if ".dev" in version:
switcher_version = "dev"
elif "rc" in version:
switcher_version = version.split("rc", maxsplit=1)[0] + " (rc)"

(the .dev part is the relevant line)
Do you know if that broke somehow?

Seems the switcher_version variable is only used for the HTML metadata. Therefore I have followed the previous implementation modifying version variable.

hkhojasteh

This comment was marked as duplicate.

@hkhojasteh hkhojasteh requested a review from lithomas1 October 2, 2023 11:52
@lithomas1
Copy link
Member

So, the issue here is that now (with the change to the new build system), the output of pd.__version__ is
'2.2.0dev0+290.gc8e7a98d6e.dirty'

Notice how there is a . missing before the dev, so the if ".dev" in version check will return False, and not change the displayed version to dev.

Can you try updating the if statement there?

@hkhojasteh
Copy link
Contributor Author

So, the issue here is that now (with the change to the new build system), the output of pd.__version__ is '2.2.0dev0+290.gc8e7a98d6e.dirty'

Notice how there is a . missing before the dev, so the if ".dev" in version check will return False, and not change the displayed version to dev.

Can you try updating the if statement there?

Definitely ;) Now you have both changes and the version fixed on both the dropdown menu and the HTML info

@jorisvandenbossche
Copy link
Member

So, the issue here is that now (with the change to the new build system), the output of pd.__version__ is '2.2.0dev0+290.gc8e7a98d6e.dirty'

Notice how there is a . missing before the dev, so the if ".dev" in version check will return False, and not change the displayed version to dev.

Actually, I think we need to fix the version instead. According to https://peps.python.org/pep-0440/ there needs to be a . to be a valid version specifier (in contrast to rc, which should not use a dot, to make it confusing).

I also assume this is not related to the switch of the build system, but just a tiny mistake in the tagging. Looking at are past tags:

$ git tag -l
...
v2.0.0
v2.0.0.dev0
v2.0.0rc0
v2.0.0rc1
v2.0.1
v2.0.2
v2.0.3
v2.1.0
v2.1.0.dev0
v2.1.0rc0
v2.1.1
v2.2.0dev0

You can see how the last tag deviates from the previous ones. I think we can still fix that by pushing a new tag on the same commit?

@hkhojasteh
Copy link
Contributor Author

hkhojasteh commented Oct 3, 2023

So, the issue here is that now (with the change to the new build system), the output of pd.__version__ is '2.2.0dev0+290.gc8e7a98d6e.dirty'
Notice how there is a . missing before the dev, so the if ".dev" in version check will return False, and not change the displayed version to dev.

Actually, I think we need to fix the version instead. According to https://peps.python.org/pep-0440/ there needs to be a . to be a valid version specifier (in contrast to rc, which should not use a dot, to make it confusing).

I also assume this is not related to the switch of the build system, but just a tiny mistake in the tagging. Looking at are past tags:

$ git tag -l
...
v2.0.0
v2.0.0.dev0
v2.0.0rc0
v2.0.0rc1
v2.0.1
v2.0.2
v2.0.3
v2.1.0
v2.1.0.dev0
v2.1.0rc0
v2.1.1
v2.2.0dev0

You can see how the last tag deviates from the previous ones. I think we can still fix that by pushing a new tag on the same commit?

Thanks for checking it out :) @lithomas1 feel free to put the tag and revert my last commit. Let me know in case of any help

@lithomas1
Copy link
Member

So, the issue here is that now (with the change to the new build system), the output of pd.__version__ is '2.2.0dev0+290.gc8e7a98d6e.dirty'
Notice how there is a . missing before the dev, so the if ".dev" in version check will return False, and not change the displayed version to dev.

Actually, I think we need to fix the version instead. According to https://peps.python.org/pep-0440/ there needs to be a . to be a valid version specifier (in contrast to rc, which should not use a dot, to make it confusing).

I also assume this is not related to the switch of the build system, but just a tiny mistake in the tagging. Looking at are past tags:

$ git tag -l
...
v2.0.0
v2.0.0.dev0
v2.0.0rc0
v2.0.0rc1
v2.0.1
v2.0.2
v2.0.3
v2.1.0
v2.1.0.dev0
v2.1.0rc0
v2.1.1
v2.2.0dev0

You can see how the last tag deviates from the previous ones. I think we can still fix that by pushing a new tag on the same commit?

Thanks for the catch !!!

I was wondering where the missing dot was coming from. This is totally on me - it looks like I botched the tag name when I repushed the 2.2.0 tag.

Interestingly, enough, the packaging package seems to recognize this as a proper version.

import pandas as pd
from packaging.version import Version
Version(pd.__version__)

gives
<Version('2.2.0.dev0+290.gc8e7a98d6e.dirty')>

I guess that's why the nightly builds version seem fine.

I'll push the new tag on Friday, sorry for the hassle all!

@lithomas1
Copy link
Member

The tag has been updated, so this should be fixed now.

I will close this PR, but thanks for helping me debug this!

@lithomas1 lithomas1 closed this Oct 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs Sprints Sprint Pull Requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DOC: fix the dev version label in the version dropdown
3 participants